1688453 - Add a test for bug 1682919

Status: RESOLVED FIXED

(Core :: Panning and Zooming, task)

Comment 1

[Botond Ballo [:botond]]

Attachment: Bug 1688453 - Support window-relative coordinates in promiseNativeClick(). r=tnikkel

Comment 2

[Botond Ballo [:botond]]

Attachment: Bug 1688453 - Add a test for bug 1682919. r=tnikkel

Depends on D103255

Comment 3

[Botond Ballo [:botond]]

I have a WIP test which passes locally with the fix for bug 1682919, and fails locally without the fix.

However, in automation it's passing without the fix too... This will need more investigation.

Comment 4

[Botond Ballo [:botond]]

After rebasing, my test no longer fails locally without the fix, so I should be able to make some progress via local debugging.

Comment 5

[Botond Ballo [:botond]]

Discovered something interesting: just clicking the scrollbar button in quick succession, vs. clicking-and-holding it, take different codepaths.

• Just clicking takes the codepath from bug 1682919 comment 6, which does the scrolling via APZ.
• With clicking-and-holding, the "holding" part takes the codepath from bug 1682919 comment 7, which does the scrolling via the main thread, and is (was) susceptible to bug 1682919.

My test currently just does clicks, so it doesn't exercise the intended codepath.

Comment 6

[Botond Ballo [:botond]]

Here is a stack trace fragment showing where the click-hold scrolling is coming from:

#0  mozilla::ScrollFrameHelper::CurPosAttributeChanged (this=0x7f15cd5d5260, aContent=0x7f15c7903310, aDoScroll=true) at /home/botond/dev/projects/mozilla/central/layout/generic/nsGfxScrollFrame.cpp:5663
#1  0x00007f15e0dc4b6b in nsHTMLScrollFrame::CurPosAttributeChanged (this=0x7f15cd5d51a8, aChild=0x7f15c7903310) at /home/botond/dev/projects/mozilla/central/layout/generic/nsGfxScrollFrame.h:1043
#2  0x00007f15e0fdeae5 in nsScrollbarFrame::AttributeChanged (this=0x7f15cd5d58c0, aNameSpaceID=0, aAttribute=0x7f15d4e20748 <mozilla::detail::gGkAtoms+63568>, aModType=1) at /home/botond/dev/projects/mozilla/central/layout/xul/nsScrollbarFrame.cpp:103
#3  0x00007f15e0fdde7c in nsScrollbarFrame::MoveToNewPosition (this=0x7f15cd5d58c0) at /home/botond/dev/projects/mozilla/central/layout/xul/nsScrollbarFrame.cpp:268
#4  0x00007f15e0d141b9 in mozilla::ScrollFrameHelper::RepeatButtonScroll (this=0x7f15cd5d5260, aScrollbar=0x7f15cd5d58c0) at /home/botond/dev/projects/mozilla/central/layout/generic/nsGfxScrollFrame.cpp:1669
#5  0x00007f15e0dc53b6 in nsHTMLScrollFrame::RepeatButtonScroll (this=0x7f15cd5d51a8, aScrollbar=0x7f15cd5d58c0) at /home/botond/dev/projects/mozilla/central/layout/generic/nsGfxScrollFrame.h:1159
#6  0x00007f15e0fde2ce in nsScrollbarButtonFrame::Notify (this=0x7f15cd5d5ca0) at /home/botond/dev/projects/mozilla/central/layout/xul/nsScrollbarButtonFrame.cpp:222
#7  0x00007f15e1009a48 in nsScrollbarButtonFrame::Notify (aData=0x7f15cd5d5ca0) at /home/botond/dev/projects/mozilla/central/layout/xul/nsScrollbarButtonFrame.h:80
#8  0x00007f15e0fd58b1 in nsRepeatService::InitTimerCallback(unsigned int)::$_1::operator()(nsITimer*, void*) const (this=0x7f15c7115480, aTimer=0x7f15c7115480, aClosure=0x0) at /home/botond/dev/projects/mozilla/central/layout/xul/nsRepeatService.cpp:87
#9  0x00007f15e0fd583d in nsRepeatService::InitTimerCallback(unsigned int)::$_1::__invoke(nsITimer*, void*) (aTimer=0x7f15c7115480, aClosure=0x0) at /home/botond/dev/projects/mozilla/central/layout/xul/nsRepeatService.cpp:76
#10 0x00007f15d9eccf2e in nsTimerImpl::Fire (this=0x7f15c71090e0, aGeneration=1) at /home/botond/dev/projects/mozilla/central/xpcom/threads/nsTimerImpl.cpp:562
#11 0x00007f15d9ecc9d0 in nsTimerEvent::Run (this=0x7f15cd9e2070) at /home/botond/dev/projects/mozilla/central/xpcom/threads/TimerThread.cpp:252

Comment 7

[Timothy Nikkel (:tnikkel)]

(In reply to Botond Ballo [:botond] from comment #6)

Here is a stack trace fragment showing where the click-hold scrolling is coming from:

FYI, I have a patch that is clean on try server that moves this scroll-from-a-scrollbar to be done like most other scroll-from-a-scrollbar. That is, moving it to use apz to do the scroll (the so called "desktop zooming" scollbar code). I'll clean it up a little, write a basic test to make sure this repeat code is at least tested for basic functionality and post it in another bug.

Comment 8

[Timothy Nikkel (:tnikkel)]

The patch I mentioned is in bug 1692997.

Comment 9

[Timothy Nikkel (:tnikkel)]

FYI, very rarely I'm able to reproduce something similar to the original bug, I filed bug 1693224.

Comment 10

[Botond Ballo [:botond]]

Attachment: Bug 1688453 - Fix include-what-you-use errors in nsScrollbarFrame.{h,cpp}. r=tnikkel

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ce22c5f9c50
Fix include-what-you-use errors in nsScrollbarFrame.{h,cpp}. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/5f71936676ba
Add a test for bug 1682919. r=tnikkel

Comment 12

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/2ce22c5f9c50
https://hg.mozilla.org/mozilla-central/rev/5f71936676ba