Commit 89bfce2d authored by Chris Lu's avatar Chris Lu Committed by Commit Bot

[ios] Read out currently selected match and words before and after match

To improve accessibility, the selectAndScrollToVisibleMatch() JavaScript
function will now also return a string containing the selected match
and words in the nodes before and after the match. This string will
then be sent out as a UIAccessibilityAnnouncementNotification.

Bug: 979707
Change-Id: I767530460b5d58c8d99bda3feb2fe29c56fb5f62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1857280
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarYi Su <mrsuyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707847}
parent e30b167b
...@@ -402,7 +402,12 @@ static NSString* gSearchTerm; ...@@ -402,7 +402,12 @@ static NSString* gSearchTerm;
- (void)findInPageManager:(web::FindInPageManager*)manager - (void)findInPageManager:(web::FindInPageManager*)manager
didSelectMatchAtIndex:(NSInteger)index didSelectMatchAtIndex:(NSInteger)index
withContextString:(NSString*)contextString
forWebState:(web::WebState*)webState { forWebState:(web::WebState*)webState {
if (contextString) {
UIAccessibilityPostNotification(UIAccessibilityAnnouncementNotification,
contextString);
}
// Increment index so that match number show in FindBar ranges from 1...N as // Increment index so that match number show in FindBar ranges from 1...N as
// opposed to 0...N-1. // opposed to 0...N-1.
index++; index++;
......
...@@ -19,6 +19,9 @@ extern const char kSelectAndScrollResultMatches[]; ...@@ -19,6 +19,9 @@ extern const char kSelectAndScrollResultMatches[];
// Dictionary key that holds value of currently selected index in // Dictionary key that holds value of currently selected index in
// kFindInPageSelectAndScrollToMatch resoonse. // kFindInPageSelectAndScrollToMatch resoonse.
extern const char kSelectAndScrollResultIndex[]; extern const char kSelectAndScrollResultIndex[];
// Dictionary key in kFindInPageSelectAndScrollToMatch response that
// holds the value of the context for the currently selected match.
extern const char kSelectAndScrollResultContextString[];
// The name of JavaScript function which stops Find in Page. // The name of JavaScript function which stops Find in Page.
extern const char kFindInPageStop[]; extern const char kFindInPageStop[];
......
...@@ -21,6 +21,8 @@ const char kSelectAndScrollResultMatches[] = "matches"; ...@@ -21,6 +21,8 @@ const char kSelectAndScrollResultMatches[] = "matches";
const char kSelectAndScrollResultIndex[] = "index"; const char kSelectAndScrollResultIndex[] = "index";
const char kSelectAndScrollResultContextString[] = "contextString";
const char kFindInPageStop[] = "findInPage.stop"; const char kFindInPageStop[] = "findInPage.stop";
} // namespace web } // namespace web
...@@ -32,13 +32,15 @@ void FindInPageManagerDelegateBridge::DidHighlightMatches(WebState* web_state, ...@@ -32,13 +32,15 @@ void FindInPageManagerDelegateBridge::DidHighlightMatches(WebState* web_state,
} }
void FindInPageManagerDelegateBridge::DidSelectMatch(WebState* web_state, void FindInPageManagerDelegateBridge::DidSelectMatch(WebState* web_state,
int index) { int index,
NSString* context_string) {
if ([delegate_ respondsToSelector:@selector if ([delegate_ respondsToSelector:@selector
(findInPageManager:didSelectMatchAtIndex:forWebState:)]) { (findInPageManager:
didSelectMatchAtIndex:withContextString:forWebState:)]) {
[delegate_ findInPageManager:web::FindInPageManager::FromWebState(web_state) [delegate_ findInPageManager:web::FindInPageManager::FromWebState(web_state)
didSelectMatchAtIndex:index didSelectMatchAtIndex:index
withContextString:context_string
forWebState:web_state]; forWebState:web_state];
} }
} }
} }
...@@ -41,9 +41,10 @@ TEST_F(FindInPageManagerDelegateBridgeTest, DidHighlightMatches) { ...@@ -41,9 +41,10 @@ TEST_F(FindInPageManagerDelegateBridgeTest, DidHighlightMatches) {
// Tests that CRWFindInPageManagerDelegate properly receives values from // Tests that CRWFindInPageManagerDelegate properly receives values from
// DidSelectMatch(). // DidSelectMatch().
TEST_F(FindInPageManagerDelegateBridgeTest, DidSelectMatch) { TEST_F(FindInPageManagerDelegateBridgeTest, DidSelectMatch) {
bridge_->DidSelectMatch(&test_web_state_, 1); bridge_->DidSelectMatch(&test_web_state_, 1, @"match context");
EXPECT_EQ(1, delegate_.index); EXPECT_EQ(1, delegate_.index);
EXPECT_EQ(&test_web_state_, delegate_.webState); EXPECT_EQ(&test_web_state_, delegate_.webState);
EXPECT_EQ(@"match context", delegate_.contextString);
} }
} // namespace web } // namespace web
...@@ -226,6 +226,7 @@ void FindInPageManagerImpl::LastFindRequestCompleted() { ...@@ -226,6 +226,7 @@ void FindInPageManagerImpl::LastFindRequestCompleted() {
} }
void FindInPageManagerImpl::SelectDidFinish(const base::Value* result) { void FindInPageManagerImpl::SelectDidFinish(const base::Value* result) {
std::string match_context_string;
if (result && result->is_dict()) { if (result && result->is_dict()) {
// Get updated match count. // Get updated match count.
const base::Value* matches = result->FindKey(kSelectAndScrollResultMatches); const base::Value* matches = result->FindKey(kSelectAndScrollResultMatches);
...@@ -246,10 +247,18 @@ void FindInPageManagerImpl::SelectDidFinish(const base::Value* result) { ...@@ -246,10 +247,18 @@ void FindInPageManagerImpl::SelectDidFinish(const base::Value* result) {
int current_index = static_cast<int>(index->GetDouble()); int current_index = static_cast<int>(index->GetDouble());
last_find_request_.SetCurrentSelectedMatchFrameIndex(current_index); last_find_request_.SetCurrentSelectedMatchFrameIndex(current_index);
} }
// Get context string.
const base::Value* context_string =
result->FindKey(kSelectAndScrollResultContextString);
if (context_string && context_string->is_string()) {
match_context_string =
static_cast<std::string>(context_string->GetString());
}
} }
if (delegate_) { if (delegate_) {
delegate_->DidSelectMatch( delegate_->DidSelectMatch(
web_state_, last_find_request_.GetCurrentSelectedMatchPageIndex()); web_state_, last_find_request_.GetCurrentSelectedMatchPageIndex(),
base::SysUTF8ToNSString(match_context_string));
} }
} }
......
...@@ -32,12 +32,17 @@ class FindInPageManagerDelegate { ...@@ -32,12 +32,17 @@ class FindInPageManagerDelegate {
int match_count, int match_count,
NSString* query) = 0; NSString* query) = 0;
// Called when a match number |index| is selected. A selected match refers to // Called when a match number |index| is selected with |context_string|
// a match that is highlighted in a unique manner different from the other // representing the text context of the match phrase. |context_string| can be
// matches. This is triggered by calling FindInPageManager::Find() with any // used in VoiceOver to notify the user of the context of the match in the
// FindInPageOptions to indicate the new match number that was selected. This // sentence. A selected match refers to a match that is
// method is not called if |FindInPageManager::Find| did not find any matches. // highlighted in a unique manner different from the other matches. This is
virtual void DidSelectMatch(WebState* web_state, int index) = 0; // triggered by calling FindInPageManager::Find() with any FindInPageOptions
// to indicate the new match number that was selected. This method is not
// called if |FindInPageManager::Find| did not find any matches.
virtual void DidSelectMatch(WebState* web_state,
int index,
NSString* context_string) = 0;
protected: protected:
virtual ~FindInPageManagerDelegate() = default; virtual ~FindInPageManagerDelegate() = default;
......
...@@ -27,13 +27,17 @@ class FindInPageManager; ...@@ -27,13 +27,17 @@ class FindInPageManager;
didHighlightMatchesOfQuery:(NSString*)query didHighlightMatchesOfQuery:(NSString*)query
withMatchCount:(NSInteger)matchCount withMatchCount:(NSInteger)matchCount
forWebState:(web::WebState*)webState; forWebState:(web::WebState*)webState;
// Called when a match number |index| is selected. A selected match refers to // Called when a match number |index| is selected with |contextString|
// a match that is highlighted in a unique manner different from the other // representing the textual context of the match. |contextString| can be used
// matches. This is triggered by calling FindInPageManager::Find() with any // in VoiceOver to notify the user of the context of the match in the sentence.
// FindInPageOptions to indicate the new match number that was selected. This // A selected match refers to a match that is highlighted in a unique manner
// method is not called if |FindInPageManager::Find| did not find any matches. // different from the other matches. This is triggered by calling
// FindInPageManager::Find() with any FindInPageOptions to indicate the new
// match number that was selected. This method is not called if
// |FindInPageManager::Find| did not find any matches.
- (void)findInPageManager:(web::FindInPageManager*)manager - (void)findInPageManager:(web::FindInPageManager*)manager
didSelectMatchAtIndex:(NSInteger)index didSelectMatchAtIndex:(NSInteger)index
withContextString:(NSString*)contextString
forWebState:(web::WebState*)webState; forWebState:(web::WebState*)webState;
@end @end
...@@ -53,7 +57,9 @@ class FindInPageManagerDelegateBridge : public web::FindInPageManagerDelegate { ...@@ -53,7 +57,9 @@ class FindInPageManagerDelegateBridge : public web::FindInPageManagerDelegate {
void DidHighlightMatches(WebState* web_state, void DidHighlightMatches(WebState* web_state,
int match_count, int match_count,
NSString* query) override; NSString* query) override;
void DidSelectMatch(WebState* web_state, int index) override; void DidSelectMatch(WebState* web_state,
int index,
NSString* context_string) override;
private: private:
__weak id<CRWFindInPageManagerDelegate> delegate_ = nil; __weak id<CRWFindInPageManagerDelegate> delegate_ = nil;
......
...@@ -25,6 +25,7 @@ class WebState; ...@@ -25,6 +25,7 @@ class WebState;
forWebState:(web::WebState*)webState; forWebState:(web::WebState*)webState;
- (void)findInPageManager:(web::FindInPageManager*)manager - (void)findInPageManager:(web::FindInPageManager*)manager
didSelectMatchAtIndex:(NSInteger)index didSelectMatchAtIndex:(NSInteger)index
withContextString:(NSString*)contextString
forWebState:(web::WebState*)webState; forWebState:(web::WebState*)webState;
// The last web::WebState received in delegate method calls. // The last web::WebState received in delegate method calls.
...@@ -35,6 +36,8 @@ class WebState; ...@@ -35,6 +36,8 @@ class WebState;
@property(nonatomic, readonly) NSInteger matchCount; @property(nonatomic, readonly) NSInteger matchCount;
// The last |index| passed in |didSelectMatchAtIndex:|. // The last |index| passed in |didSelectMatchAtIndex:|.
@property(nonatomic, readonly) NSInteger index; @property(nonatomic, readonly) NSInteger index;
// The last |contextString| passed in |didSelectMatchAtIndex:|.
@property(nonatomic, readonly) NSString* contextString;
@end @end
......
...@@ -21,8 +21,10 @@ ...@@ -21,8 +21,10 @@
- (void)findInPageManager:(web::FindInPageManager*)manager - (void)findInPageManager:(web::FindInPageManager*)manager
didSelectMatchAtIndex:(NSInteger)index didSelectMatchAtIndex:(NSInteger)index
withContextString:(NSString*)contextString
forWebState:(web::WebState*)webState { forWebState:(web::WebState*)webState {
_webState = webState; _webState = webState;
_contextString = [contextString copy];
_index = index; _index = index;
} }
......
...@@ -24,7 +24,9 @@ class FakeFindInPageManagerDelegate : public FindInPageManagerDelegate { ...@@ -24,7 +24,9 @@ class FakeFindInPageManagerDelegate : public FindInPageManagerDelegate {
void DidHighlightMatches(WebState* web_state, void DidHighlightMatches(WebState* web_state,
int match_count, int match_count,
NSString* query) override; NSString* query) override;
void DidSelectMatch(WebState* web_state, int index) override; void DidSelectMatch(WebState* web_state,
int index,
NSString* context_string) override;
// Holds the state passed to DidHighlightMatches and DidSelectMatch. // Holds the state passed to DidHighlightMatches and DidSelectMatch.
struct State { struct State {
...@@ -34,6 +36,7 @@ class FakeFindInPageManagerDelegate : public FindInPageManagerDelegate { ...@@ -34,6 +36,7 @@ class FakeFindInPageManagerDelegate : public FindInPageManagerDelegate {
int match_count = -1; int match_count = -1;
NSString* query; NSString* query;
int index = -1; int index = -1;
NSString* context_string;
}; };
// Returns the current State. // Returns the current State.
......
...@@ -28,12 +28,14 @@ void FakeFindInPageManagerDelegate::DidHighlightMatches(WebState* web_state, ...@@ -28,12 +28,14 @@ void FakeFindInPageManagerDelegate::DidHighlightMatches(WebState* web_state,
} }
void FakeFindInPageManagerDelegate::DidSelectMatch(WebState* web_state, void FakeFindInPageManagerDelegate::DidSelectMatch(WebState* web_state,
int index) { int index,
NSString* context_string) {
if (!delegate_state_) { if (!delegate_state_) {
delegate_state_ = std::make_unique<State>(); delegate_state_ = std::make_unique<State>();
} }
delegate_state_->web_state = web_state; delegate_state_->web_state = web_state;
delegate_state_->index = index; delegate_state_->index = index;
delegate_state_->context_string = context_string;
} }
} // namespace web } // namespace web
...@@ -198,9 +198,10 @@ TEST_F(FindInPageJsTest, FindAcrossMultipleNodes) { ...@@ -198,9 +198,10 @@ TEST_F(FindInPageJsTest, FindAcrossMultipleNodes) {
})); }));
} }
// Tests that a FindInPage match can be highlighted. // Tests that a FindInPage match can be highlighted and the correct
// accessibility string is returned.
TEST_F(FindInPageJsTest, FindHighlightMatch) { TEST_F(FindInPageJsTest, FindHighlightMatch) {
ASSERT_TRUE(LoadHtml("<span>foo</span>")); ASSERT_TRUE(LoadHtml("<span>some foo match</span>"));
const base::TimeDelta kCallJavascriptFunctionTimeout = const base::TimeDelta kCallJavascriptFunctionTimeout =
base::TimeDelta::FromSeconds(kWaitForJSCompletionTimeout); base::TimeDelta::FromSeconds(kWaitForJSCompletionTimeout);
ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{ ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{
...@@ -225,12 +226,15 @@ TEST_F(FindInPageJsTest, FindHighlightMatch) { ...@@ -225,12 +226,15 @@ TEST_F(FindInPageJsTest, FindHighlightMatch) {
})); }));
__block bool highlight_done = false; __block bool highlight_done = false;
__block std::string context_string;
std::vector<base::Value> highlight_params; std::vector<base::Value> highlight_params;
highlight_params.push_back(base::Value(0)); highlight_params.push_back(base::Value(0));
main_web_frame()->CallJavaScriptFunction( main_web_frame()->CallJavaScriptFunction(
kFindInPageSelectAndScrollToMatch, highlight_params, kFindInPageSelectAndScrollToMatch, highlight_params,
base::BindOnce(^(const base::Value* result) { base::BindOnce(^(const base::Value* result) {
highlight_done = true; highlight_done = true;
context_string =
result->FindKey(kSelectAndScrollResultContextString)->GetString();
}), }),
kCallJavascriptFunctionTimeout); kCallJavascriptFunctionTimeout);
ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{ ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{
...@@ -240,12 +244,13 @@ TEST_F(FindInPageJsTest, FindHighlightMatch) { ...@@ -240,12 +244,13 @@ TEST_F(FindInPageJsTest, FindHighlightMatch) {
EXPECT_NSEQ(@1, EXPECT_NSEQ(@1,
ExecuteJavaScript( ExecuteJavaScript(
@"document.getElementsByClassName('find_selected').length")); @"document.getElementsByClassName('find_selected').length"));
EXPECT_EQ("some foo match", context_string);
} }
// Tests that a FindInPage match can be highlighted and that a previous // Tests that a FindInPage match can be highlighted and that a previous
// highlight is removed when another match is highlighted. // highlight is removed when another match is highlighted.
TEST_F(FindInPageJsTest, FindHighlightSeparateMatches) { TEST_F(FindInPageJsTest, FindHighlightSeparateMatches) {
ASSERT_TRUE(LoadHtml("<span>foo foo</span>")); ASSERT_TRUE(LoadHtml("<span>foo foo match</span>"));
const base::TimeDelta kCallJavascriptFunctionTimeout = const base::TimeDelta kCallJavascriptFunctionTimeout =
base::TimeDelta::FromSeconds(kWaitForJSCompletionTimeout); base::TimeDelta::FromSeconds(kWaitForJSCompletionTimeout);
ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{ ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{
...@@ -270,18 +275,22 @@ TEST_F(FindInPageJsTest, FindHighlightSeparateMatches) { ...@@ -270,18 +275,22 @@ TEST_F(FindInPageJsTest, FindHighlightSeparateMatches) {
})); }));
__block bool highlight_done = false; __block bool highlight_done = false;
__block std::string context_string;
std::vector<base::Value> highlight_params; std::vector<base::Value> highlight_params;
highlight_params.push_back(base::Value(0)); highlight_params.push_back(base::Value(0));
main_web_frame()->CallJavaScriptFunction( main_web_frame()->CallJavaScriptFunction(
kFindInPageSelectAndScrollToMatch, highlight_params, kFindInPageSelectAndScrollToMatch, highlight_params,
base::BindOnce(^(const base::Value* result) { base::BindOnce(^(const base::Value* result) {
highlight_done = true; highlight_done = true;
context_string =
result->FindKey(kSelectAndScrollResultContextString)->GetString();
}), }),
kCallJavascriptFunctionTimeout); kCallJavascriptFunctionTimeout);
ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{ ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{
return highlight_done; return highlight_done;
})); }));
EXPECT_EQ("foo ", context_string);
EXPECT_NSEQ(@1, EXPECT_NSEQ(@1,
ExecuteJavaScript( ExecuteJavaScript(
@"document.getElementsByClassName('find_selected').length")); @"document.getElementsByClassName('find_selected').length"));
...@@ -293,12 +302,15 @@ TEST_F(FindInPageJsTest, FindHighlightSeparateMatches) { ...@@ -293,12 +302,15 @@ TEST_F(FindInPageJsTest, FindHighlightSeparateMatches) {
kFindInPageSelectAndScrollToMatch, highlight_second_params, kFindInPageSelectAndScrollToMatch, highlight_second_params,
base::BindOnce(^(const base::Value* result) { base::BindOnce(^(const base::Value* result) {
highlight_done = true; highlight_done = true;
context_string =
result->FindKey(kSelectAndScrollResultContextString)->GetString();
}), }),
kCallJavascriptFunctionTimeout); kCallJavascriptFunctionTimeout);
ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{ ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{
return highlight_done; return highlight_done;
})); }));
EXPECT_EQ(" foo match", context_string);
id inner_html = ExecuteJavaScript(@"document.body.innerHTML"); id inner_html = ExecuteJavaScript(@"document.body.innerHTML");
ASSERT_TRUE([inner_html isKindOfClass:[NSString class]]); ASSERT_TRUE([inner_html isKindOfClass:[NSString class]]);
EXPECT_TRUE([inner_html EXPECT_TRUE([inner_html
......
...@@ -668,10 +668,37 @@ __gCrWeb.findInPage.selectAndScrollToVisibleMatch = function(index) { ...@@ -668,10 +668,37 @@ __gCrWeb.findInPage.selectAndScrollToVisibleMatch = function(index) {
selectedMatchIndex_ = total_match_index; selectedMatchIndex_ = total_match_index;
selectedVisibleMatchIndex_ = index; selectedVisibleMatchIndex_ = index;
getCurrentSelectedMatch_().addSelectHighlight(); match = getCurrentSelectedMatch_();
match.addSelectHighlight();
scrollToCurrentlySelectedMatch_(); scrollToCurrentlySelectedMatch_();
return {matches: visibleMatchCount, index: index}; // Get string consisting of the text contents of the match nodes and the
// nodes before and after them, if applicable.
// This will be read out as an accessibility notification for the match.
// The siblings's textContent are added into the node array, because the
// nextSibling and previousSibling properties to the match nodes sometimes
// are text nodes, not HTML nodes. This results in '[object Text]' string
// being added to the array instead of the object.
let nodes = match.nodes.slice();
if (match.nodes[0].previousSibling) {
nodes.unshift([match.nodes[0].previousSibling.textContent]);
}
if (match.nodes[match.nodes.length-1].nextSibling) {
nodes.push([match.nodes[match.nodes.length-1].nextSibling.textContent]);
}
let contextString = nodes.map(function(node) {
if (node.textContent) {
return node.textContent;
} else {
return node;
}
}).join("");
return {
matches: visibleMatchCount,
index: index,
contextString: contextString
};
}; };
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment