RESOLVED FIXED 192303
[iOS] Handle hit testing for subframes 
https://bugs.webkit.org/show_bug.cgi?id=192303
Summary [iOS] Handle hit testing for subframes
Frédéric Wang (:fredw)
Reported 2018-12-03 00:34:58 PST
Extracting the windowToContents/contentToWindow changes from bug 173833. These are necessary to properly handle hit testing.
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-
DetailsFormatted DiffDiff
patch (11.65 KB, patch)
2019-01-21 06:06 PST, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Frédéric Wang (:fredw)
Comment 1 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]
Simon Fraser (smfr)
Comment 2 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()
Frédéric Wang (:fredw)
Comment 3 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.
Antti Koivisto
Comment 4 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).
Frédéric Wang (:fredw)
Comment 5 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).
Antti Koivisto
Comment 6 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.
Antti Koivisto
Comment 7 2019-01-21 06:06:05 PST
Created attachment 359686 [details] patch
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2019-01-21 22:50:56 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 10 2019-01-21 22:51:29 PST
<rdar://problem/47438192>
Frédéric Wang (:fredw)
Comment 11 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.
Antti Koivisto
Comment 12 2019-05-09 07:54:29 PDT
*** Bug 197613 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.