Bug 198657 - Limit simulated mouse events on Google Maps to entering Street View
Summary: Limit simulated mouse events on Google Maps to entering Street View
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-06-07 05:35 PDT by Antoine Quint
Modified: 2019-07-18 05:52 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.48 KB, patch)
2019-06-07 05:36 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (2.45 KB, patch)
2019-06-07 05:41 PDT, Antoine Quint
bfulgham: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2019-06-07 05:35:59 PDT
Limit simulated mouse events on Google Maps to entering Street View
Comment 1 Antoine Quint 2019-06-07 05:36:19 PDT
Created attachment 371584 [details]
Patch
Comment 2 Antoine Quint 2019-06-07 05:36:24 PDT
<rdar://problem/51345064>
Comment 3 EWS Watchlist 2019-06-07 05:39:05 PDT
Attachment 371584 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Antoine Quint 2019-06-07 05:41:02 PDT
Created attachment 371585 [details]
Patch
Comment 5 Brent Fulgham 2019-06-07 09:56:43 PDT
Comment on attachment 371585 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=371585&action=review

> Source/WebCore/page/Quirks.cpp:336
> +#endif

In file included from /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource321.cpp:4:
./page/Quirks.cpp:324:70: error: unused parameter 'target' [-Werror,-Wunused-parameter]
bool Quirks::shouldDispatchSimulatedMouseEventsOnTarget(EventTarget* target) const
                                                                     ^
1 error generated.

You need an UNUSED_PARAM(target) here.
Comment 6 Brent Fulgham 2019-06-07 09:57:13 PDT
Comment on attachment 371585 [details]
Patch

r=me with the fix I suggested.
Comment 7 Antoine Quint 2019-06-07 09:58:42 PDT
(In reply to Brent Fulgham from comment #5)
> Comment on attachment 371585 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=371585&action=review
> 
> > Source/WebCore/page/Quirks.cpp:336
> > +#endif
> 
> In file included from
> /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-
> sources/UnifiedSource321.cpp:4:
> ./page/Quirks.cpp:324:70: error: unused parameter 'target'
> [-Werror,-Wunused-parameter]
> bool Quirks::shouldDispatchSimulatedMouseEventsOnTarget(EventTarget* target)
> const
>                                                                      ^
> 1 error generated.
> 
> You need an UNUSED_PARAM(target) here.

Yeah, I figured :) But instead I'm going to make the two Quirk methods related to simulated mouse events only defined for PLATFORM(IOS_FAMILY) since it really doesn't make sense to ever call those on another platform.
Comment 8 Antoine Quint 2019-06-07 10:11:10 PDT
Committed r246205: <https://trac.webkit.org/changeset/246205>
Comment 9 Michael Catanzaro 2019-06-07 15:18:45 PDT
Committed r246216: <https://trac.webkit.org/changeset/246216>
Comment 10 Michael Catanzaro 2019-06-07 15:23:28 PDT
Ah apologies Brent, I improperly pasted a Reviewed By line with your name in my build fix, but that is not correct because you didn't review my patch. Either I forgot to remove that line, or maybe (I suspect) webkit-patch added it in. (I don't think it used to do that. Or perhaps it did it because I forgot to write "unreviewed" in the commit message.)

Anyway, for the record, Brent didn't review r246216.
Comment 11 Masataka Yakura 2019-06-08 07:58:28 PDT
https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/page/Quirks.cpp?rev=246205#L331

> if (equalLettersIgnoringASCIICase(host, "www.google.com") && url.path().startsWithIgnoringASCIICase("/maps/"))

Unlike other Google properties, people can access Google Maps using localized URLs (e.g. https://www.google.co.jp/maps , https://www.google.de/maps , https://www.google.com.hk/maps , and bunch more) and Google doesn't redirect them to google.com so if this quirk is not specific to the US version, we'll have trouble in non-US countries.
Comment 12 Antoine Quint 2019-07-18 05:52:20 PDT
(In reply to Masataka Yakura from comment #11)
> https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/page/Quirks.
> cpp?rev=246205#L331
> 
> > if (equalLettersIgnoringASCIICase(host, "www.google.com") && url.path().startsWithIgnoringASCIICase("/maps/"))
> 
> Unlike other Google properties, people can access Google Maps using
> localized URLs (e.g. https://www.google.co.jp/maps ,
> https://www.google.de/maps , https://www.google.com.hk/maps , and bunch
> more) and Google doesn't redirect them to google.com so if this quirk is not
> specific to the US version, we'll have trouble in non-US countries.

Thanks Masataka, I filed https://bugs.webkit.org/show_bug.cgi?id=199904 to track that issue.