Commit 6943fdf8 authored by Eugene But's avatar Eugene But Committed by Commit Bot

Use -userIsInteracting for user gesture detection if voice over is off.

Notable changes:
 - updated testLinkWithBlankTargetWithoutUserGesture to execute script
   without user gesture
 - removed NavigationActionInitiationType::kScriptInitiated because there
   is no way to tell if action was initiated by the script (coordinates or
   SyntheticClickType can be 0 for valid link clicks).
 - Updated webView:createWebViewWithConfiguration: to use
   GetNavigationActionInitiationTypeWithVoiceOverOn, because there is no
   need to inspect action.description when Voice over is Off.
   -userIsInteracting can give false positives for link clicks, which is
   ok. window.open also have these false positives, so if the page wants
   to open a popup, it can exploit window.open.

Bug: 849749
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I8c179c9df2d6bce3253e46272c2f14e0eb6224bc
Reviewed-on: https://chromium-review.googlesource.com/1087389
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: default avatarMohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565442}
parent 44663e8f
...@@ -28,6 +28,7 @@ ...@@ -28,6 +28,7 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
using chrome_test_util::ExecuteJavaScript;
using chrome_test_util::GetCurrentWebState; using chrome_test_util::GetCurrentWebState;
using chrome_test_util::OmniboxText; using chrome_test_util::OmniboxText;
using chrome_test_util::TapWebViewElementWithId; using chrome_test_util::TapWebViewElementWithId;
...@@ -86,8 +87,6 @@ id<GREYMatcher> PopupBlocker() { ...@@ -86,8 +87,6 @@ id<GREYMatcher> PopupBlocker() {
// Tests that sessionStorage content is available for windows opened by DOM via // Tests that sessionStorage content is available for windows opened by DOM via
// target="_blank" links. // target="_blank" links.
- (void)testLinkWithBlankTargetSessionStorage { - (void)testLinkWithBlankTargetSessionStorage {
using chrome_test_util::ExecuteJavaScript;
NSError* error = nil; NSError* error = nil;
ExecuteJavaScript(@"sessionStorage.setItem('key', 'value');", &error); ExecuteJavaScript(@"sessionStorage.setItem('key', 'value');", &error);
GREYAssert(!error, @"Error during script execution: %@", error); GREYAssert(!error, @"Error during script execution: %@", error);
...@@ -113,8 +112,12 @@ id<GREYMatcher> PopupBlocker() { ...@@ -113,8 +112,12 @@ id<GREYMatcher> PopupBlocker() {
// Tests executing script that clicks a link with target="_blank". // Tests executing script that clicks a link with target="_blank".
- (void)testLinkWithBlankTargetWithoutUserGesture { - (void)testLinkWithBlankTargetWithoutUserGesture {
GREYAssert(TapWebViewElementWithId("webScenarioWindowOpenRegularLink"), chrome_test_util::SetContentSettingsBlockPopups(CONTENT_SETTING_BLOCK);
@"Failed to tap \"webScenarioWindowOpenRegularLink\""); NSError* error = nil;
ExecuteJavaScript(
@"document.getElementById('webScenarioWindowOpenRegularLink').click()",
&error);
GREYAssert(!error, @"Failed to tap 'webScenarioWindowOpenRegularLink'");
[ChromeEarlGrey waitForElementWithMatcherSufficientlyVisible:PopupBlocker()]; [ChromeEarlGrey waitForElementWithMatcherSufficientlyVisible:PopupBlocker()];
[ChromeEarlGrey waitForMainTabCount:1]; [ChromeEarlGrey waitForMainTabCount:1];
} }
......
...@@ -4089,21 +4089,19 @@ registerLoadRequestForURL:(const GURL&)requestURL ...@@ -4089,21 +4089,19 @@ registerLoadRequestForURL:(const GURL&)requestURL
GURL openerURL = GURL openerURL =
referrer.length ? GURL(base::SysNSStringToUTF8(referrer)) : _documentURL; referrer.length ? GURL(base::SysNSStringToUTF8(referrer)) : _documentURL;
bool initiatedByUser = web::GetNavigationActionInitiationType(action) == // There is no reliable way to tell if there was a user gesture, so this code
// checks if user has recently tapped on web view. TODO(crbug.com/809706):
// Remove the usage of -userIsInteracting when rdar://19989909 is fixed.
bool initiatedByUser = [self userIsInteracting];
if (UIAccessibilityIsVoiceOverRunning()) {
// -userIsInteracting returns NO if VoiceOver is On. Inspect action's
// description, which may contain the information about user gesture for
// certain link clicks.
initiatedByUser = initiatedByUser ||
web::GetNavigationActionInitiationTypeWithVoiceOverOn(
action.description) ==
web::NavigationActionInitiationType::kUserInitiated; web::NavigationActionInitiationType::kUserInitiated;
if (action.navigationType == WKNavigationTypeLinkActivated) {
// Link with target="_blank" navigation. |initiatedByUser| reliably
// indicates the presence of the user gesture.
} else {
DCHECK(action.navigationType == WKNavigationTypeOther ||
action.navigationType == WKNavigationTypeFormSubmitted)
<< "unexpected navigation type: " << action.navigationType;
// Form submission with target="_blank" or window.open() navigation (with or
// without user gesture). There is no reliable way to tell if there was a
// user gesture, so this code checks if user has recently tapped on web
// view. TODO(crbug.com/809706): Remove the usage of -userIsInteracting when
// rdar://19989909 is fixed.
initiatedByUser = initiatedByUser || [self userIsInteracting];
} }
WebState* childWebState = WebState* childWebState =
......
...@@ -17,10 +17,8 @@ enum class NavigationActionInitiationType { ...@@ -17,10 +17,8 @@ enum class NavigationActionInitiationType {
// there is no way to detect if the navigationAction initiator by examining // there is no way to detect if the navigationAction initiator by examining
// the WKNavigationAction fields. // the WKNavigationAction fields.
kUnknownInitiator = 0, kUnknownInitiator = 0,
// The navigation action is initiated by the user. // The navigation action is a link click initiated by the user.
kUserInitiated, kUserInitiated,
// The navigation action is initiated by by page script.
kScriptInitiated,
}; };
// Returns the WKNavigationAction initiation type. // Returns the WKNavigationAction initiation type.
......
...@@ -28,6 +28,7 @@ NavigationActionInitiationType GetNavigationActionInitiationType( ...@@ -28,6 +28,7 @@ NavigationActionInitiationType GetNavigationActionInitiationType(
// Example description where the syntheticClickType is set: // Example description where the syntheticClickType is set:
// "<classname: 0x123124; navigationType = 2; syntheticClickType = 1; // "<classname: 0x123124; navigationType = 2; syntheticClickType = 1;
// position x = 90.20 y = 10.29 request = null; ..>" // position x = 90.20 y = 10.29 request = null; ..>"
// SyntheticClickType still can be 0 for user-initiated actions.
NavigationActionInitiationType NavigationActionInitiationType
GetNavigationActionInitiationTypeWithVoiceOverOff( GetNavigationActionInitiationTypeWithVoiceOverOff(
NSString* action_description) { NSString* action_description) {
...@@ -49,7 +50,7 @@ GetNavigationActionInitiationTypeWithVoiceOverOff( ...@@ -49,7 +50,7 @@ GetNavigationActionInitiationTypeWithVoiceOverOff(
// TwoFingerTap}. // TwoFingerTap}.
int click_type = [action_description substringWithRange:match_range].intValue; int click_type = [action_description substringWithRange:match_range].intValue;
return click_type ? NavigationActionInitiationType::kUserInitiated return click_type ? NavigationActionInitiationType::kUserInitiated
: NavigationActionInitiationType::kScriptInitiated; : NavigationActionInitiationType::kUnknownInitiator;
} }
// Returns the NavigationAction initiation type based on the string description // Returns the NavigationAction initiation type based on the string description
...@@ -60,6 +61,7 @@ GetNavigationActionInitiationTypeWithVoiceOverOff( ...@@ -60,6 +61,7 @@ GetNavigationActionInitiationTypeWithVoiceOverOff(
// Example description where only position is set: // Example description where only position is set:
// "<classname: 0x121124; navigationType = 1; syntheticClickType = 0; // "<classname: 0x121124; navigationType = 1; syntheticClickType = 0;
// position x = 90.20 y = 10.29 request = null; ..>" // position x = 90.20 y = 10.29 request = null; ..>"
// Both coordinates still can be 0 for user-initiated actions.
NavigationActionInitiationType GetNavigationActionInitiationTypeWithVoiceOverOn( NavigationActionInitiationType GetNavigationActionInitiationTypeWithVoiceOverOn(
NSString* action_description) { NSString* action_description) {
NSRegularExpression* position_regex = [NSRegularExpression NSRegularExpression* position_regex = [NSRegularExpression
...@@ -86,7 +88,7 @@ NavigationActionInitiationType GetNavigationActionInitiationTypeWithVoiceOverOn( ...@@ -86,7 +88,7 @@ NavigationActionInitiationType GetNavigationActionInitiationTypeWithVoiceOverOn(
.floatValue; .floatValue;
return (position_y || position_x) return (position_y || position_x)
? NavigationActionInitiationType::kUserInitiated ? NavigationActionInitiationType::kUserInitiated
: NavigationActionInitiationType::kScriptInitiated; : NavigationActionInitiationType::kUnknownInitiator;
} }
} // namespace web } // namespace web
...@@ -40,13 +40,13 @@ TEST_F(WKNavigationActionUtilTest, InitiationTypeFromDescriptionWithVoiceOver) { ...@@ -40,13 +40,13 @@ TEST_F(WKNavigationActionUtilTest, InitiationTypeFromDescriptionWithVoiceOver) {
NavigationActionInitiationType::kUnknownInitiator; NavigationActionInitiationType::kUnknownInitiator;
initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOn( initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOn(
MakeDescriptionString(0, 0, 0, 0)); MakeDescriptionString(0, 0, 0, 0));
EXPECT_EQ(NavigationActionInitiationType::kScriptInitiated, initiation_type); EXPECT_EQ(NavigationActionInitiationType::kUnknownInitiator, initiation_type);
initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOn( initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOn(
MakeDescriptionString(0, 0, 100.0, 25.0)); MakeDescriptionString(0, 0, 100.0, 25.0));
EXPECT_EQ(NavigationActionInitiationType::kUserInitiated, initiation_type); EXPECT_EQ(NavigationActionInitiationType::kUserInitiated, initiation_type);
initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOn( initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOn(
MakeDescriptionString(0, 1, 0, 0)); MakeDescriptionString(0, 1, 0, 0));
EXPECT_EQ(NavigationActionInitiationType::kScriptInitiated, initiation_type); EXPECT_EQ(NavigationActionInitiationType::kUnknownInitiator, initiation_type);
initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOn( initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOn(
MakeDescriptionString(2, 2, 20.0, 30.4)); MakeDescriptionString(2, 2, 20.0, 30.4));
EXPECT_EQ(NavigationActionInitiationType::kUserInitiated, initiation_type); EXPECT_EQ(NavigationActionInitiationType::kUserInitiated, initiation_type);
...@@ -63,10 +63,10 @@ TEST_F(WKNavigationActionUtilTest, InitiationTypeFromDescriptionNoVoiceOver) { ...@@ -63,10 +63,10 @@ TEST_F(WKNavigationActionUtilTest, InitiationTypeFromDescriptionNoVoiceOver) {
NavigationActionInitiationType::kUnknownInitiator; NavigationActionInitiationType::kUnknownInitiator;
initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOff( initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOff(
MakeDescriptionString(0, 0, 0, 0)); MakeDescriptionString(0, 0, 0, 0));
EXPECT_EQ(NavigationActionInitiationType::kScriptInitiated, initiation_type); EXPECT_EQ(NavigationActionInitiationType::kUnknownInitiator, initiation_type);
initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOff( initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOff(
MakeDescriptionString(0, 0, 100.0, 25.0)); MakeDescriptionString(0, 0, 100.0, 25.0));
EXPECT_EQ(NavigationActionInitiationType::kScriptInitiated, initiation_type); EXPECT_EQ(NavigationActionInitiationType::kUnknownInitiator, initiation_type);
initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOff( initiation_type = GetNavigationActionInitiationTypeWithVoiceOverOff(
MakeDescriptionString(2, 1, 0, 0)); MakeDescriptionString(2, 1, 0, 0));
EXPECT_EQ(NavigationActionInitiationType::kUserInitiated, initiation_type); EXPECT_EQ(NavigationActionInitiationType::kUserInitiated, initiation_type);
......
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