Bug 194813 - [iOS] Correctly handle overlapping regions for elements with a touch-action property
Summary: [iOS] Correctly handle overlapping regions for elements with a touch-action p...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on: 196608 196701
Blocks:
  Show dependency tree / graph
 
Reported: 2019-02-19 01:55 PST by Antoine Quint
Modified: 2019-05-09 01:34 PDT (History)
3 users (show)

See Also:

Attachments
wip (11.89 KB, patch)
2019-04-08 05:59 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
wip (12.45 KB, patch)
2019-04-08 07:00 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
Patch (30.60 KB, patch)
2019-05-08 12:42 PDT, Antoine Quint 		koivisto: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews213 for win-future (13.65 MB, application/zip)
2019-05-08 15:48 PDT, EWS Watchlist 		no flags Details
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2019-02-19 01:55:40 PST 
The code we currently use to gather regions for elements with touch-action set on them is very basic and does not account for any element that might overlap it. For instance, having "touch-action: none" on an element and having another element overlap in a different hierarchy means the other element is also treated to have "touch-action: none".
Comment 1 Radar WebKit Bug Importer 2019-02-19 02:20:01 PST 
<rdar://problem/48194708>
Comment 2 Antti Koivisto 2019-04-08 05:59:25 PDT 
Created attachment 366929 [details]
wip
Comment 3 Antti Koivisto 2019-04-08 07:00:37 PDT 
Created attachment 366932 [details]
wip
Comment 4 Antti Koivisto 2019-04-08 12:10:09 PDT 
Moved the part that computes the touch actions from remote layer tree to bug 196701 for review.
Comment 5 Antoine Quint 2019-05-07 03:22:19 PDT 
Found an issue where elements with "display: inline-block" don't have a touch-action region, see https://bugs.webkit.org/show_bug.cgi?id=197281.
Comment 6 Antoine Quint 2019-05-08 12:42:00 PDT 
Created attachment 369410 [details]
Patch
Comment 7 EWS Watchlist 2019-05-08 15:48:32 PDT 
Comment on attachment 369410 [details]
Patch

Attachment 369410 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12136954

New failing tests:
security/contentSecurityPolicy/video-with-file-url-allowed-by-media-src-star.html
legacy-animation-engine/compositing/reflections/load-video-in-reflection.html
Comment 8 EWS Watchlist 2019-05-08 15:48:34 PDT 
Created attachment 369438 [details]
Archive of layout-test-results from ews213 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews213  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 9 Antti Koivisto 2019-05-09 00:40:55 PDT 
Comment on attachment 369410 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1342
> +            if (!touchActions.contains(WebCore::TouchAction::PinchZoom))
> +                _webView.scrollView.pinchGestureRecognizer.enabled = NO;

Where does this thing get reset?
Comment 10 Antoine Quint 2019-05-09 01:30:30 PDT 
(In reply to Antti Koivisto from comment #9)
> Comment on attachment 369410 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=369410&action=review
> 
> r=me
> 
> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1342
> > +            if (!touchActions.contains(WebCore::TouchAction::PinchZoom))
> > +                _webView.scrollView.pinchGestureRecognizer.enabled = NO;
> 
> Where does this thing get reset?

I'm not actually sure, but testing indicates that the value is reset as expected. Maybe UIKit performs that reset internally, I couldn't find any code in WebKit that would do this.
Comment 11 Antoine Quint 2019-05-09 01:34:41 PDT 
Committed r245112: <https://trac.webkit.org/changeset/245112>