[iOS] Basic hit testing for content overlapping fast-scrollable overflow

• Component: Scrolling
• Status: RESOLVED FIXED

[Antti Koivisto]

Better UI side hit testing.

[Antti Koivisto]

Created attachment 363739 [details]
patch

[Sam Weinig]

Comment on attachment 363739 [details]
patch

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

> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:77
> +    for (auto i = viewsAtPoint.size(); i--;) {

I know this doesn't really have to do with this bug, this makes me sad I still haven't added the reversed adaptor :(.  I think all it would end up being is something like:

template<typename Container>
IteratorRange<typename Container::reverse_iterator> makeReversedIteratorRange(Container& container)
{
    return makeIteratorRange(std::rbegin(container), std::rend(container));
}

template<typename Container>
IteratorRange<typename Container::const_reverse_iterator> makeReversedIteratorRange(const Container& container)
{
    return makeIteratorRange(std::crbegin(container), std::crend(container));
}


and then this loop would become:

for (auto *view : makeReversedIteratorRange(viewsAtPoint)) {

Do you think that's worth adding?

[EWS Watchlist]

Comment on attachment 363739 [details]
patch

Attachment 363739 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11397079

New failing tests:
fast/scrolling/ios/overflow-scroll-overlap.html

[EWS Watchlist]

Created attachment 363748 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Antti Koivisto]

> for (auto *view : makeReversedIteratorRange(viewsAtPoint)) {
> 
> Do you think that's worth adding?

Yes!

[Simon Fraser (smfr)]

Comment on attachment 363739 [details]
patch

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

I think the first version of this patch needs at least to have a a bit that comes from the web process that says "does this layer contain some content that should participate in hit testing". It should lay the groundwork for the region-based approach (I don't think we can ship without that).

> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:52
> +            if (view.layer.contents)
> +                return false;

We can make layer with no contents for empty divs (see isSimpleContainerCompositingLayer()), yet they should still intercept hit testing. In addition, there may be other views in the hierarchy which are there for structural purposes and should not participate in hit testing (e.g. a GraphicsLayer's m_structuralLayer).

I think you need to actually send data from the web process about which layers are hit-testable (and, in future, that data would include a region).

Also, a GraphicsLayer can paint one or more renderers which don't cover its entire area. Trivially, you can do this with box-shadow: 0 0 20px black to add layer padding that should not hit test. It can also happen with nested positioned elements.

Also, elements with pointer-events: none need to be ignored.

[Antti Koivisto]

Created attachment 363752 [details]
patch

[Antti Koivisto]

> We can make layer with no contents for empty divs (see
> isSimpleContainerCompositingLayer()), yet they should still intercept hit
> testing. In addition, there may be other views in the hierarchy which are
> there for structural purposes and should not participate in hit testing
> (e.g. a GraphicsLayer's m_structuralLayer).
> 
> I think you need to actually send data from the web process about which
> layers are hit-testable (and, in future, that data would include a region).
> 
> Also, a GraphicsLayer can paint one or more renderers which don't cover its
> entire area. Trivially, you can do this with box-shadow: 0 0 20px black to
> add layer padding that should not hit test. It can also happen with nested
> positioned elements.
> 
> Also, elements with pointer-events: none need to be ignored.

That'S why there is a FIXME for more coverage.

[Simon Fraser (smfr)]

Comment on attachment 363752 [details]
patch

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

> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:45
> +        // FIXME: This probably doesn't cover all possible cases.

We know this doesn't cover all cases.

[Antti Koivisto]

Created attachment 363762 [details]
patch

[WebKit Commit Bot]

Comment on attachment 363762 [details]
patch

Clearing flags on attachment: 363762

Committed r242564: <https://trac.webkit.org/changeset/242564>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48649132>