[webkit-reviews] review denied: [Bug 195360] [iOS] Hit test for content overlapping fast-scrollable overflow : [Attachment 363739] patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Antti Koivisto
<koivisto at iki.fi>'s request for review:
Bug 195360: [iOS] Hit test for content overlapping fast-scrollable overflow
https://bugs.webkit.org/show_bug.cgi?id=195360

Attachment 363739: patch

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




--- Comment #6 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 363739
  --> https://bugs.webkit.org/attachment.cgi?id=363739
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.