UI-process hit-testing needs to know about containing block relationships

• Component: Scrolling
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Created attachment 364917 [details]
Simple testcase

When we hit-test in the UI process, hit-testing via the UIView hierarchy isn't always correct, because that reflects painting order, not hit-testing order.

This matters when overflow:scroll contains a composited layer (say, position:relative) whose stacking context is outside the overflow. You can't scroll the overflow:scroll if you happen to hit this box.

This breaks scrolling at https://palace-games.com:

1. Load https://palace-games.com/ on iPad
2. Tap “Reserve a room today”
3. Try to scroll the list of rooms; try touching in different places. Sometimes the overflow doesn't scroll, and you rubber-band the body instead.

[Radar WebKit Bug Importer]

<rdar://problem/48949633>

[Simon Fraser (smfr)]

On macOS, what happens is that hit-testing finds the position:relative box via a reverse z-order tree walk, then via event propagation up the DOM, the element with overflow:scroll handles the event.

[Antti Koivisto]

Scrolling tree needs to know scrolling relationships so we can probably just ask it.

[Antti Koivisto]

Created attachment 365346 [details]
wip

[EWS Watchlist]

Attachment 365346 [details] did not pass style-queue:


ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:85:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:86:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Antti Koivisto]

Created attachment 365526 [details]
patch

[EWS Watchlist]

Attachment 365526 [details] did not pass style-queue:


ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:85:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:86:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 2 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 365526 [details]
patch

Attachment 365526 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11596959

New failing tests:
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html

[EWS Watchlist]

Created attachment 365532 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Antti Koivisto]

Created attachment 365536 [details]
patch

[EWS Watchlist]

Attachment 365536 [details] did not pass style-queue:


ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:85:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:86:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 2 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Antti Koivisto]

Created attachment 365540 [details]
patch

[EWS Watchlist]

Attachment 365540 [details] did not pass style-queue:


ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:85:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:86:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 2 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 365540 [details]
patch

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

> Source/WebKit/ChangeLog:11
> +        When an overflow scroller contains a positioned element it may not be a descendant layer of the scroller,

The "it" is ambiguous.

> Source/WebKit/UIProcess/RemoteLayerTree/RemoteScrollingCoordinatorProxy.cpp:83
> -    std::unique_ptr<ScrollingStateTree> stateTree(const_cast<RemoteScrollingCoordinatorTransaction&>(transaction).scrollingStateTree().release());
> +    auto stateTree = WTFMove(const_cast<RemoteScrollingCoordinatorTransaction&>(transaction).scrollingStateTree());

Haha

> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm:133
> +    // FIXME: This doesn't contain ScrollPositioningBehavior::Stationary nodes. They may need to be handled too.

File bug for this, reference it here.

> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteScrollingCoordinatorProxyIOS.mm:139
> +            auto* positionedLayerNode = RemoteLayerTreeNode::forCALayer(positionedNode->layer());

Maybe null-check positionedNode, since we've had other bugs where stale nodeIDs get left around.

[Antti Koivisto]

Created attachment 365588 [details]
patch

[Antti Koivisto]

> File bug for this, reference it here.

https://bugs.webkit.org/show_bug.cgi?id=196100

[EWS Watchlist]

Attachment 365588 [details] did not pass style-queue:


ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:85:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:86:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 2 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 365588 [details]
patch

Clearing flags on attachment: 365588

Committed r243316: <https://trac.webkit.org/changeset/243316>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.