Bug 192303 - [iOS] Handle hit testing for subframes
Summary: [iOS] Handle hit testing for subframes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords: InRadar
: 197613 (view as bug list)
Depends on: 173833
Blocks: 196394 149264 182868 194900
  Show dependency treegraph
 
Reported: 2018-12-03 00:34 PST by Frédéric Wang (:fredw)
Modified: 2019-05-09 07:54 PDT (History)
6 users (show)

See Also:


Attachments
Patch (applies on top of bug 173833) (7.47 KB, patch)
2018-12-03 00:54 PST, Frédéric Wang (:fredw)
simon.fraser: review+
simon.fraser: commit-queue-
Details | Formatted Diff | Diff
patch (11.65 KB, patch)
2019-01-21 06:06 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2018-12-03 00:34:58 PST
Extracting the windowToContents/contentToWindow changes from bug 173833. These are necessary to properly handle hit testing.
Comment 1 Frédéric Wang (:fredw) 2018-12-03 00:54:48 PST
Created attachment 356365 [details]
Patch (applies on top of bug 173833)

This has been extracted from attachment 356214 [details]
Comment 2 Simon Fraser (smfr) 2018-12-03 14:01:19 PST
Comment on attachment 356365 [details]
Patch (applies on top of bug 173833)

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

> Source/WebCore/page/FrameView.h:682
> +    bool coordinateChangeShouldIgnoreScrollPosition() const final;

Please rename this to windowToContentsShouldIgnoreScrollPosition()
Comment 3 Frédéric Wang (:fredw) 2018-12-03 14:52:39 PST
Comment on attachment 356365 [details]
Patch (applies on top of bug 173833)

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

>> Source/WebCore/page/FrameView.h:682
>> +    bool coordinateChangeShouldIgnoreScrollPosition() const final;
> 
> Please rename this to windowToContentsShouldIgnoreScrollPosition()

smfr: I can but note that as I said elsewhere the function might be needed in other siutations:

https://bugs.webkit.org/attachment.cgi?id=353085&action=review
https://bugs.webkit.org/attachment.cgi?id=348915&action=review

Anyway, this is blocked by bug 173833.
Comment 4 Antti Koivisto 2019-01-19 01:59:44 PST
Comment on attachment 356365 [details]
Patch (applies on top of bug 173833)

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

Want to land this?

> Source/WebCore/platform/ScrollView.cpp:933
> -    if (delegatesScrolling())
> +    if (coordinateChangeShouldIgnoreScrollPosition())

This change is needed in a bunch of other places too (IntRect, rootView versions versions of these function).
Comment 5 Frédéric Wang (:fredw) 2019-01-19 04:22:59 PST
Comment on attachment 356365 [details]
Patch (applies on top of bug 173833)

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

@Antti: Feel free to land the code changes if you want to.

>> Source/WebCore/platform/ScrollView.cpp:933
>> +    if (coordinateChangeShouldIgnoreScrollPosition())
> 
> This change is needed in a bunch of other places too (IntRect, rootView versions versions of these function).

Yes, but I tried to keep this minimal and focused as Simon had concerned about changing all these functions. See also comment 3 for similar bugs (involving other functions).
Comment 6 Antti Koivisto 2019-01-21 01:29:45 PST
Actually I think this is wrong. The delegatesScrolling bit shouldn't be set for subframes in the first place. It is meant for application controlled top level scrolling only.
Comment 7 Antti Koivisto 2019-01-21 06:06:05 PST
Created attachment 359686 [details]
patch
Comment 8 WebKit Commit Bot 2019-01-21 22:50:54 PST
Comment on attachment 359686 [details]
patch

Clearing flags on attachment: 359686

Committed r240249: <https://trac.webkit.org/changeset/240249>
Comment 9 WebKit Commit Bot 2019-01-21 22:50:56 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2019-01-21 22:51:29 PST
<rdar://problem/47438192>
Comment 11 Frédéric Wang (:fredw) 2019-01-22 00:53:48 PST
> 15:29:48 - anttik: fredw: note that the test is missing the programmatic scroll subtest, that needs an additional fix
> 15:30:46 - fredw: anttik: ok. Yes I guess we need the stuff from bug 182868 for programmatic scrolling

Will move the remaining test into bug 182868.
Comment 12 Antti Koivisto 2019-05-09 07:54:29 PDT
*** Bug 197613 has been marked as a duplicate of this bug. ***