The page 'jumps' when selecting text in a zoomed in state
Categories
(Core :: Panning and Zooming, defect, P2)
Tracking
()
People
(Reporter: Laurentiu.Apahidean, Assigned: botond)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Steps to reproduce:
- In about:config set "layout.scroll.root-frame-containers" to "0".
- Go to a website that supports zooming in and text selection (Ex. https://en.wikipedia.org/wiki/Mozilla)
- Zoom in the page by double taping or pinch zoom.
- Long tap a word in order to select it.
Actual results:
The page focus shifts.
Expected results:
The word is selected and the page remains in the same position.
Notes: for more details please see the attached video.
Assignee | ||
Comment 1•6 years ago
|
||
The layer for the text selection handles carries RCD-RSF scroll metadata, but is not a descendant of the async zoom container. That's going to upset all sorts of APZ assumptions.
cc Markus
Assignee | ||
Comment 2•6 years ago
|
||
Here is a stack trace for the creation of a display item for the selection carets:
#0 nsDisplayItem::nsDisplayItem (this=0x7fc4db1ca420, aBuilder=0x7fc4db3ba000, aFrame=0x7fc4e182e528, aActiveScrolledRoot=0x7fc4e369cb00, aAnonymous=false)
at /home/botond/dev/projects/mozilla/central/layout/painting/nsDisplayList.cpp:3077
#1 0x00007fc4f158ae73 in nsDisplayHitTestInfoItem::nsDisplayHitTestInfoItem (this=0x7fc4db1ca420, aBuilder=0x7fc4db3ba000, aFrame=0x7fc4e182e528,
aActiveScrolledRoot=0x7fc4e369cb00, aAnonymous=false) at /home/botond/dev/projects/mozilla/central/layout/painting/nsDisplayList.h:3603
#2 0x00007fc4f155f0fb in nsDisplayWrapList::nsDisplayWrapList (this=0x7fc4db1ca420, aBuilder=0x7fc4db3ba000, aFrame=0x7fc4e182e528, aList=0x7ffd49a0f160,
aActiveScrolledRoot=0x7fc4e369cb00, aClearClipChain=true, aIndex=0, aAnonymous=false)
at /home/botond/dev/projects/mozilla/central/layout/painting/nsDisplayList.cpp:5492
#3 0x00007fc4f12cd78f in MakeDisplayItem<nsDisplayWrapList, nsIFrame*&, nsDisplayList*&, mozilla::ActiveScrolledRoot const*&, bool>(nsDisplayListBuilder*, nsIFrame*&, nsDisplayList*&, mozilla::ActiveScrolledRoot const*&, bool&&) (aBuilder=0x7fc4db3ba000,
at /home/botond/dev/projects/mozilla/central/layout/painting/nsDisplayList.h:2030
#4 0x00007fc4f12114d0 in WrapInWrapList (aBuilder=0x7fc4db3ba000, aFrame=0x7fc4e182e528, aList=0x7ffd49a0f160, aContainerASR=0x7fc4e369cb00, aCanSkipWrapList=false)
at /home/botond/dev/projects/mozilla/central/layout/generic/nsFrame.cpp:3395
#5 0x00007fc4f1210bb8 in nsIFrame::BuildDisplayListForChild (this=0x7fc4e182e3b8, aBuilder=0x7fc4db3ba000, aChild=0x7fc4e182e5e0, aLists=..., aFlags=4)
at /home/botond/dev/projects/mozilla/central/layout/generic/nsFrame.cpp:3794
#6 0x00007fc4f11ab7fe in DisplayLine (aBuilder=0x7fc4db3ba000, aLineArea=..., aLine=..., aDepth=0, aDrawnLines=@0x7ffd49a0f7ec: 32708, aLists=..., aFrame=0x7fc4e182e3b8,
aTextOverflow=0x0, aLineNumberForTextOverflow=0) at /home/botond/dev/projects/mozilla/central/layout/generic/nsBlockFrame.cpp:6405
#7 0x00007fc4f11aac96 in nsBlockFrame::BuildDisplayList (this=0x7fc4e182e3b8, aBuilder=0x7fc4db3ba000, aLists=...)
at /home/botond/dev/projects/mozilla/central/layout/generic/nsBlockFrame.cpp:6496
#8 0x00007fc4f120cc7f in nsIFrame::BuildDisplayListForStackingContext (this=0x7fc4e182e3b8, aBuilder=0x7fc4db3ba000, aList=0x7ffd49a109b0,
aCreatedContainerItem=0x7ffd49a108d7) at /home/botond/dev/projects/mozilla/central/layout/generic/nsFrame.cpp:3061
#9 0x00007fc4f12105c0 in nsIFrame::BuildDisplayListForChild (this=0x7fc4e1ef0448, aBuilder=0x7fc4db3ba000, aChild=0x7fc4db3f7de8, aLists=..., aFlags=4)
at /home/botond/dev/projects/mozilla/central/layout/generic/nsFrame.cpp:3711
#10 0x00007fc4f11ab7fe in DisplayLine (aBuilder=0x7fc4db3ba000, aLineArea=..., aLine=..., aDepth=0, aDrawnLines=@0x7ffd49a1103c: 32708, aLists=..., aFrame=0x7fc4e1ef0448,
aTextOverflow=0x0, aLineNumberForTextOverflow=0) at /home/botond/dev/projects/mozilla/central/layout/generic/nsBlockFrame.cpp:6405
#11 0x00007fc4f11aac96 in nsBlockFrame::BuildDisplayList (this=0x7fc4e1ef0448, aBuilder=0x7fc4db3ba000, aLists=...)
at /home/botond/dev/projects/mozilla/central/layout/generic/nsBlockFrame.cpp:6496
#12 0x00007fc4f120cc7f in nsIFrame::BuildDisplayListForStackingContext (this=0x7fc4e1ef0448, aBuilder=0x7fc4db3ba000, aList=0x7ffd49a11ff8, aCreatedContainerItem=0x0)
at /home/botond/dev/projects/mozilla/central/layout/generic/nsFrame.cpp:3061
#13 0x00007fc4f1185d15 in BuildDisplayListForTopLayerFrame (aBuilder=0x7fc4db3ba000, aFrame=0x7fc4e1ef0448, aList=0x7ffd49a122c8)
at /home/botond/dev/projects/mozilla/central/layout/generic/ViewportFrame.cpp:124
#14 0x00007fc4f11859be in mozilla::ViewportFrame::BuildDisplayListForTopLayer (this=0x7fc4dee8d020, aBuilder=0x7fc4db3ba000, aList=0x7ffd49a122c8)
at /home/botond/dev/projects/mozilla/central/layout/generic/ViewportFrame.cpp:180
#15 0x00007fc4f1185334 in mozilla::ViewportFrame::BuildDisplayList (this=0x7fc4dee8d020, aBuilder=0x7fc4db3ba000, aLists=...)
at /home/botond/dev/projects/mozilla/central/layout/generic/ViewportFrame.cpp:64
#16 0x00007fc4f120cc7f in nsIFrame::BuildDisplayListForStackingContext (this=0x7fc4dee8d020, aBuilder=0x7fc4db3ba000, aList=0x7ffd49a13008, aCreatedContainerItem=0x0)
at /home/botond/dev/projects/mozilla/central/layout/generic/nsFrame.cpp:3061
#17 0x00007fc4f1525508 in RetainedDisplayListBuilder::AttemptPartialUpdate (this=0x7fc4db3ba000, aBackstop=0, aChecker=0x0)
at /home/botond/dev/projects/mozilla/central/layout/painting/RetainedDisplayListBuilder.cpp:1304
#18 0x00007fc4f10ecfcf in nsLayoutUtils::PaintFrame (aRenderingContext=0x0, aFrame=0x7fc4dee8d020, aDirtyRegion=..., aBackstop=0,
aBuilderMode=nsDisplayListBuilderMode::PAINTING,
aFlags=(nsLayoutUtils::PaintFrameFlags::PAINT_WIDGET_LAYERS | nsLayoutUtils::PaintFrameFlags::PAINT_EXISTING_TRANSACTION | nsLayoutUtils::PaintFrameFlags::PAINT_NO_COMP
The ASR that gets assigned to the item (which is the RCD-RSF's ASR, hence the layer getting the RCD-RSF metadata) gets pushed onto the display list builder in BuildDisplayListForTopLayerFrame()
.
Updated•6 years ago
|
Comment 3•6 years ago
|
||
I am seeing some bad behavior when in this zoomed in state besides the viewport changes. When in the state described by the bug I am not able to pan to the right to the selected text. Pinch zooming is broken. Pressing the device's back button allows the user to escape this broken state. https://www.youtube.com/watch?v=DFXONZHhZcI
Assignee | ||
Comment 4•6 years ago
|
||
Kevin: to clarify, you are testing with layout.scroll.root-frame-containers=0 (which is not the default yet)?
If so, then yes, based on the diagnosis in comment 1, I would expect all sorts of brokenness in the described state.
Comment 5•6 years ago
|
||
Yes I changed the pref. Is 0 the planned default?
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Kevin Brosnan [:kbrosnan] from comment #5)
Is 0 the planned default?
Yes, but only after this bug is fixed :)
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Comment 7•6 years ago
|
||
Yes, it sounds like the top layer needs to be moved inside the async zoom container. This can probably be achieved by doing the work that's currently done by ViewportFrame::BuildDisplayList
somewhere in ScrollFrameHelper::BuildDisplayList
instead, for root scroll frames. (Or maybe (equivalently?) for scroll frames that are viewport frames.)
At the moment, the top layer is on top of the root scroll frame's scroll bars. I think it would make sense to change that, and move it just underneath the scroll bars. Unless that means that, for example on YouTube, fullscreen video would display scrollbars from the page... I don't think it would though.
It may also make sense to build the display items for the top layer while the root scroll frame's asrSetter is on the stack, since top layer elements such as the carets or devtools highlighters can be scrolled. Actually, I don't really understand how that works at all, at the moment - where are those scrolled top layer items getting the right ASR from, if they're created after the root scroll frame's asrSetter has been popped from the stack?
Comment 8•6 years ago
|
||
In Chrome, which supports the dialog
element and its ::backdrop
pseudo element, I was able to achieve a scrolled element inside the top layer by setting position: absolute on the dialog's backdrop: https://position-absolute-backdrop.glitch.me (remix at https://glitch.com/edit/#!/position-absolute-backdrop )
Chrome places the scrollbars on top of that dialog backdrop.
The Firefox implementation of dialog::backdrop
was happening in bug 1322939 but then stalled.
Assignee | ||
Comment 9•6 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #7)
It may also make sense to build the display items for the top layer while the root scroll frame's asrSetter is on the stack, since top layer elements such as the carets or devtools highlighters can be scrolled. Actually, I don't really understand how that works at all, at the moment - where are those scrolled top layer items getting the right ASR from, if they're created after the root scroll frame's asrSetter has been popped from the stack?
I believe I answered that at the bottom of comment 2:
The ASR that gets assigned to the item (which is the RCD-RSF's ASR, hence the layer getting the RCD-RSF metadata) gets pushed onto the display list builder in BuildDisplayListForTopLayerFrame().
Assignee | ||
Comment 10•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 11•6 years ago
|
||
Assignee | ||
Comment 12•6 years ago
|
||
This maintains the important invariant that layers that carry scroll metadata
for the RCD-RSF are inside the async zoom container.
Updated•6 years ago
|
Comment 13•6 years ago
|
||
Comment 14•6 years ago
|
||
bugherder |
Reporter | ||
Comment 15•6 years ago
|
||
Hello,
I tested this issue on the latest version of Nightly 67.0a1 (2019-03-13) with Sony Xperia Z3 (Android 5.1.1), Samsung Galaxy S8+ (Android 8.0.0), Huawei MediaPad M3 Lite 10 Android 7.0 and I wasn't able to reproduce the issue.
Due to my findings, I'll mark this issue as verified.
Description
•