Closed
Bug 1076447
Opened 10 years ago
Closed 10 years ago
Visible region for overlay scrollbars gets truncated when APZ zooms content
Categories
(Core :: Layout, defect)
Tracking
()
People
(Reporter: kats, Assigned: tnikkel)
References
Details
Attachments
(1 file, 1 obsolete file)
7.99 KB,
patch
|
mattwoodrow
:
review+
bajaj
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This is a spinoff bug from bug 1043859. What's happening (with that bug applied) is if you go to a page like https://staktrace.com/spout/entry.php?id=824 (or any zoomable page) in the B2G browser, zoom in a little bit, and scroll down to the bottom, the scrollbars disappear. If you scroll down very slowly you can see that once you get near the bottom of the page the scrollbars get clipped by an increasingly large amount and eventually get clipped out entirely. The layers dump indicates that the visible region on the scrollbar layer (computed by the layout code) is getting shortened for some reason. See bug 1043859 comment 19, 20, 21, 22, 25 for some additional discussion on this in which I prove again how little I know about layout code :)
Reporter | ||
Comment 1•10 years ago
|
||
[Blocking Requested - why for this release]: root-level scrollbars in browser do not render completely in some areas after zooming in, which looks pretty unpolished. the root-level scrollbars were not present at all in releases prior to 2.1; they were added in bug 995519. Bug 1043859 fixes one problem with the scrollbars, and this is a follow-up bug to fix another problem.
blocking-b2g: --- → 2.1?
status-b2g-v2.0:
--- → unaffected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → affected
Reporter | ||
Updated•10 years ago
|
Blocks: gfx-target-2.1
Comment 2•10 years ago
|
||
Just to note that you can also see this when the page is scrolled to the right. Horizontal scrollbar will be clipped gradually, and vertical scrollbar will disappear.
Assignee | ||
Comment 3•10 years ago
|
||
The dirty rect for root scroll frames will already get set to the display port by nsSubDocumentFrame or nsLayoutUtils::PaintFrame, so the dirty rect passed in to the scroll frame will already be the display port. There is no reason the display port needs to contain the scroll bars. So should just include all of the scroll bars in the dirty in this case.
Attachment #8502323 -
Flags: review?(matt.woodrow)
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8502323 [details] [diff] [review] adjust the dirty rect for scroll bars This patch makes all the scrollbars disappear entirely on my Flame.
Attachment #8502323 -
Flags: feedback-
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8502323 [details] [diff] [review] adjust the dirty rect for scroll bars Review of attachment 8502323 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsGfxScrollFrame.cpp @@ +2563,5 @@ > nsDisplayListBuilder::AutoCurrentScrollbarInfoSetter > infoSetter(aBuilder, scrollTargetId, flags); > nsDisplayListCollection partList; > mOuter->BuildDisplayListForChild( > + aBuilder, scrollParts[i], dirty, partList, The patch works as intended if I replace this usage of "dirty" with "aUsingDisplayPort ? scrollParts[i]->GetVisualOverflowRectRelativeToParent() : aDirtyRect" because "dirty" here is relative to scrollParts[i] but this function expects it to be relative to the parent.
Attachment #8502323 -
Flags: feedback- → feedback+
Updated•10 years ago
|
blocking-b2g: 2.1? → 2.1+
Comment 6•10 years ago
|
||
I tested with applying all related 2.1 patches from http://people.mozilla.org/~kgupta/tmp/scrollbar/ on flame, and I could not reproduced this issue, and scroll functionality looks good as well.
Assignee | ||
Comment 7•10 years ago
|
||
Thanks for testing kats. I fixed that in this patch.
Attachment #8502323 -
Attachment is obsolete: true
Attachment #8502323 -
Flags: review?(matt.woodrow)
Attachment #8502713 -
Flags: review?(matt.woodrow)
Comment 8•10 years ago
|
||
Comment on attachment 8502713 [details] [diff] [review] adjust the dirty rect for scroll bars Review of attachment 8502713 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsGfxScrollFrame.cpp @@ +2558,5 @@ > + // include all of the scrollbars if we have a display port. > + nsRect dirty = aUsingDisplayPort ? > + scrollParts[i]->GetVisualOverflowRectRelativeToParent() : aDirtyRect; > + nsDisplayListBuilder::AutoBuildingDisplayList > + buildingForChild(aBuilder, scrollParts[i], I'm not sure that we need this, since BuildDisplayListForChild does it too.
Attachment #8502713 -
Flags: review?(matt.woodrow) → review+
Assignee | ||
Comment 9•10 years ago
|
||
I mentioned the reason for that tersely in patch commit comment. We need the dirty rect on the display list builder to be set correctly for when AppendToTop creates an nsDisplayWrapList or nsDisplayOwnLayer item containing the scrollbar items using the scrollbar as the underlying frame.
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d88161338f3
https://hg.mozilla.org/mozilla-central/rev/9d88161338f3
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Reporter | ||
Comment 12•10 years ago
|
||
Comment on attachment 8502713 [details] [diff] [review] adjust the dirty rect for scroll bars Approval Request Comment [Feature/regressing bug #]: bug 995519 [User impact if declined]: root frame scrollbars (such as the page scrollbars in the browser) get clipped after zooming in, or are hidden entirely. [Describe test coverage new/current, TBPL]: tested by no-jun on 2.1 [Risks and why]: the code is shared with desktop but the specific code paths should only be used on B2G and metro. fairly low risk. [String/UUID change made/needed]: none
Attachment #8502713 -
Flags: approval-mozilla-aurora?
Reporter | ||
Updated•10 years ago
|
Updated•10 years ago
|
Attachment #8502713 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 14•10 years ago
|
||
please verifyme on 2.1 after it gets uplifted and landed. Await comments here (and flag: status-b2g-2.1 ==> fixed) to track when the patch lands.
Comment 15•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/0120fba88a61
Comment 16•10 years ago
|
||
Timothy, I am looking to verify this bug, however, I'm seeing an issue with the horizontal scroll bar in Flame 2.1 (Full Flash, nightly). If what I'm seeing is intended behaviour, please needinfo me, so I can verify (ddixon@qanalydocs.com). Actual Results: The horizontal scroll bar is clipped out of view when user zooms in, scrolls to bottom then up again. Video: https://www.youtube.com/watch?v=uPsnx4T6SAE&feature=youtu.be Note: This issue seems to only occur on certain web pages such as Google image search. Device: Flame 2.1 Build ID: 20141017001201 Gaia: 1ea74943cfe525c76a074ca1d7de8e51a70f6b98 Gecko: 2befa902ff5c Version: 34.0 (2.1) Firmware Version: v180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Flags: needinfo?(tnikkel)
Comment 17•10 years ago
|
||
(In reply to Duane Dixon [:ddixon] from comment #16) > Timothy, > > I am looking to verify this bug, however, I'm seeing an issue with the > horizontal scroll bar in Flame 2.1 (Full Flash, nightly). If what I'm seeing > is intended behaviour, please needinfo me, so I can verify > (ddixon@qanalydocs.com). > > Actual Results: The horizontal scroll bar is clipped out of view when user > zooms in, scrolls to bottom then up again. > > Video: > https://www.youtube.com/watch?v=uPsnx4T6SAE&feature=youtu.be > > Note: This issue seems to only occur on certain web pages such as Google > image search. > > Device: Flame 2.1 > Build ID: 20141017001201 > Gaia: 1ea74943cfe525c76a074ca1d7de8e51a70f6b98 > Gecko: 2befa902ff5c > Version: 34.0 (2.1) > Firmware Version: v180 > User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 kats explained it to me that this is due to how the new chrome browser is implemented. when you scroll down, it shows the title bar of the browser, and this results in hiding the horizontal scroll bar. So horizontal scroll bar is only visible if you haven't scrolled down.
Reporter | ||
Comment 18•10 years ago
|
||
Yeah, what No-Jun said. I'll run this by the UX folks to make sure it's acceptable behaviour. Regardless it would be a different bug if we need to change anything there.
Flags: needinfo?(tnikkel)
Comment 19•10 years ago
|
||
Based on comment 17, this issue is verified fixed in Flame 2.2 (Full Flash, nightly). Actual Results: Scroll bars in Browser function correctly when user zooms-in scrolls to the bottom of a web page. Device: Flame 2.2 BuildID: 20141103040202 Gaia: bc168c17474dabbcceaa349e9bc7c95654435aec Gecko: 5999e92e89ff Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 36.0a1 (2.2) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0 Device: Flame 2.1 BuildID: 20141103001220 Gaia: 027a7de0c95320cea0579bfd1a4ceef3e9038f34 Gecko: ffecb2be228b Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 34.0 (2.1) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: verifyme
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in
before you can comment on or make changes to this bug.
Description
•