Bug 158439 - Allow passive mouse wheel event listeners to not force synchronous scrolling
Summary: Allow passive mouse wheel event listeners to not force synchronous scrolling
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
: 217676 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-06-06 15:35 PDT by Simon Fraser (smfr)
Modified: 2020-10-21 22:13 PDT (History)
13 users (show)

See Also:


Attachments
Patch (52.61 KB, patch)
2020-10-13 15:30 PDT, Simon Fraser (smfr)
thorton: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2016-06-06 15:35:22 PDT
Since bug 149466 is fixed, we should avoid synchronous scrolling for pages with passive wheel event handlers.
Comment 1 Radar WebKit Bug Importer 2016-09-12 13:30:41 PDT
<rdar://problem/28265360>
Comment 2 Simon Fraser (smfr) 2020-10-13 15:30:46 PDT
Created attachment 411270 [details]
Patch
Comment 3 Sam Weinig 2020-10-13 15:33:23 PDT
Comment on attachment 411270 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Turn on ENABLE_WHEEL_EVENT_REGIONS for macOS, and hit-test the passive and active regions

This might totally not be feasible at this stage, but is this something we can do with a runtime switch rather than an #ifdef?

> Source/WebCore/page/Frame.cpp:319
>      // FIXME: This needs to look at wheel event handlers too.

Is this comment still needed?
Comment 4 Simon Fraser (smfr) 2020-10-13 15:35:21 PDT
(In reply to Sam Weinig from comment #3)
> Comment on attachment 411270 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=411270&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        Turn on ENABLE_WHEEL_EVENT_REGIONS for macOS, and hit-test the passive and active regions
> 
> This might totally not be feasible at this stage, but is this something we
> can do with a runtime switch rather than an #ifdef?

I plan to remove the #ifdef. This could be behind a switch, but I'm not sure it needs to be?

> > Source/WebCore/page/Frame.cpp:319
> >      // FIXME: This needs to look at wheel event handlers too.
> 
> Is this comment still needed?

No, I resolved it.
Comment 5 Sam Weinig 2020-10-13 16:16:59 PDT
(In reply to Simon Fraser (smfr) from comment #4)
> (In reply to Sam Weinig from comment #3)
> > Comment on attachment 411270 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=411270&action=review
> > 
> > > Source/WebCore/ChangeLog:9
> > > +        Turn on ENABLE_WHEEL_EVENT_REGIONS for macOS, and hit-test the passive and active regions
> > 
> > This might totally not be feasible at this stage, but is this something we
> > can do with a runtime switch rather than an #ifdef?
> 
> I plan to remove the #ifdef. This could be behind a switch, but I'm not sure
> it needs to be?

No switch and no #ifdef is the best outcome, so I am happy.

> 
> > > Source/WebCore/page/Frame.cpp:319
> > >      // FIXME: This needs to look at wheel event handlers too.
> > 
> > Is this comment still needed?
> 
> No, I resolved it.

Remove it then!
Comment 6 Simon Fraser (smfr) 2020-10-14 11:06:53 PDT
https://trac.webkit.org/changeset/268476/webkit
Comment 7 Hector Lopez 2020-10-14 14:57:58 PDT
editing/editable-region/iframe.html

Test is a failing according to history on iOS. First occurrence of failure is at r268476.

History:

https://results.webkit.org/?suite=layout-tests&test=editing%2Feditable-region%2Fiframe.html

By the diff it looks as though a re-baseline for result to accommodate the difference in width of 10

Diff:
--- /Volumes/Data/slave/ipados-simulator-14-release-tests-wk2/build/layout-test-results/editing/editable-region/iframe-expected.txt
+++ /Volumes/Data/slave/ipados-simulator-14-release-tests-wk2/build/layout-test-results/editing/editable-region/iframe-actual.txt
@@ -33,7 +33,7 @@
                               (bounds 310.00 670.00)
                               (drawsContent 1)
                               (event region
-                                (rect (0,0) width=300 height=670)
+                                (rect (0,0) width=310 height=670)
                               (editable region
                                 (rect (8,286) width=284 height=24)
                                 (rect (9,431) width=285 height=24)
Comment 8 Hector Lopez 2020-10-14 15:14:54 PDT
Re-baseline for test editing/editable-region/iframe.html

https://trac.webkit.org/changeset/268491/webkit
Comment 9 Simon Fraser (smfr) 2020-10-20 15:31:08 PDT
*** Bug 217676 has been marked as a duplicate of this bug. ***