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)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla35
blocking-b2g 2.1+
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
b2g-v2.0 --- unaffected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: kats, Assigned: tnikkel)

References

Details

Attachments

(1 file, 1 obsolete file)

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 :)
[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?
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.
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)
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-
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+
blocking-b2g: 2.1? → 2.1+
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.
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 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+
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.
https://hg.mozilla.org/mozilla-central/rev/9d88161338f3
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
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?
Attachment #8502713 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Lets also verify this once it lands on 2.1.
Keywords: verifyme
No longer blocks: 1078316
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.
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)
(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.
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)
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
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Depends on: 1214261
You need to log in before you can comment on or make changes to this bug.