Bug 211522 - visualViewport.addEventListener("scroll"*** fires permanently after zoom or orientation change
Summary: visualViewport.addEventListener("scroll"*** fires permanently after zoom or o...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: Safari 13
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-06 11:41 PDT by juwagn
Modified: 2020-07-02 13:35 PDT (History)
8 users (show)

See Also:


Attachments
requested attachment (2.38 KB, text/html)
2020-05-06 23:13 PDT, juwagn
no flags Details
Patch (4.82 KB, patch)
2020-07-02 13:06 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (4.90 KB, patch)
2020-07-02 13:14 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description juwagn 2020-05-06 11:41:36 PDT
Hello,

in latest version 13.4.1 adding such code
visualViewport.addEventListener("scroll", function(q) { ***** etc.
function fires permanently after zoom or orientation change. It fires it approximately ~5 times a second continuously without stop.
Not even page refresh helps to stop firing, after page refresh it continues to fire scroll event non-stop!

Sincerely yours.
Comment 1 Radar WebKit Bug Importer 2020-05-06 11:41:51 PDT
<rdar://problem/62939371>
Comment 2 Simon Fraser (smfr) 2020-05-06 16:07:54 PDT
Can you attach a testcase, or point to a URL that shows the issue? Thanks.
Comment 3 juwagn 2020-05-06 23:13:40 PDT
Created attachment 398706 [details]
requested attachment

Reproducer case.

1. on iOS 13.4.1 Safari open viewportScroll.html
2. zoom in 

now "scroll" event fires continuously non stop
You can zoom out again, that doesn't matter, it will fire this event continuously. If you don't zoom out back but just refresh the page then issue continues too.
Comment 4 juwagn 2020-05-06 23:15:08 PDT
zoom-in with two finger gesture
Comment 5 Simon Fraser (smfr) 2020-05-07 10:40:51 PDT
Thank you.
Comment 6 Simon Fraser (smfr) 2020-05-20 07:54:25 PDT
Also reproduces on macOS.
Comment 7 Simon Fraser (smfr) 2020-05-20 08:06:11 PDT
Stupid coding error:

    if (m_needsVisualViewportScrollEvent) {
        LOG_WITH_STREAM(Events, stream << "Document" << this << "sending scroll events to visualViewport");
        m_needsVisualViewportResizeEvent = false;

Should be m_needsVisualViewportScrollEvent = false.

Also all the double comparisons in VisualViewport::update() should probably be a bit more lenient.
Comment 8 Antoine Quint 2020-07-02 13:06:41 PDT
Created attachment 403390 [details]
Patch
Comment 9 Simon Fraser (smfr) 2020-07-02 13:09:26 PDT
Comment on attachment 403390 [details]
Patch

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

> LayoutTests/fast/visual-viewport/scroll-event-fired-during-scroll-alone.html:36
> +    }, "Verify that a scroll event gets fired on window.visualViewport when its offset changes");

This text doesn't seem right. You're verifying that only one scroll event fires.
Comment 10 Antoine Quint 2020-07-02 13:11:26 PDT
(In reply to Simon Fraser (smfr) from comment #9)
> Comment on attachment 403390 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403390&action=review
> 
> > LayoutTests/fast/visual-viewport/scroll-event-fired-during-scroll-alone.html:36
> > +    }, "Verify that a scroll event gets fired on window.visualViewport when its offset changes");
> 
> This text doesn't seem right. You're verifying that only one scroll event
> fires.

Oops, leftover from a test I copied.
Comment 11 Antoine Quint 2020-07-02 13:14:38 PDT
Created attachment 403392 [details]
Patch for landing
Comment 12 EWS 2020-07-02 13:35:19 PDT
Committed r263858: <https://trac.webkit.org/changeset/263858>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403392 [details].