Closed Bug 976370 Opened 6 years ago Closed 6 years ago

don't let scrollbars interfere with the creation of a scrollable layer

Categories

(Core :: Layout, defect)

29 Branch
x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30
blocking-b2g 1.3+
Tracking Status
firefox28 --- wontfix
firefox29 --- wontfix
firefox30 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
If the only thing stopping scroll layer items from being merged is a scrollbar item for the same scroll frame we could (should?) just move the scrollbar up to be directly on top of the scroll layer item containing the scrolled content. This is a more desirably position for the scrollbar anyway, and it will let us create a scrollable layer. Note that the attached patch will also move a scrollbar that is directly below a scroll layer item that has merged fully successfully to be directly above it.

If the patch for bug 965593 is relanded we will need this so that it doesn't cause regressions like bug 975221.
Attachment #8381024 - Flags: review?(roc)
Comment on attachment 8381024 [details] [diff] [review]
patch

Review of attachment 8381024 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsDisplayList.cpp
@@ +1003,5 @@
>    }
>    return opaqueClipped;
>  }
>  
> +/* Checks if aPotenialScrollItem is a scroll layer item and

s/aPotential/aPotential/, here and the actual variable name
Blocks a 1.3+ bug, so requesting 1.3+ on this too.
blocking-b2g: --- → 1.3?
blocking-b2g: 1.3? → 1.3+
I'm not sure how to write a reftest for this since overlay scrollbars fadeaway, so any test would fail some of the time if enough time past for the scrollbars to start fading after we make them visible by scrolling.
https://hg.mozilla.org/mozilla-central/rev/4d1674345259
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Please request approval-mozilla-b2g28 for this patch for v1.3 uplift.
Flags: needinfo?(tnikkel)
Comment on attachment 8381024 [details] [diff] [review]
patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

Scrollbars are currently z-index: max 32bit int; on 1.3 so this will have very limited effects if any on 1.3 until bug 965593 is landed. But I think it's better to get this in sooner rather than later.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 965593
User impact if declined: slow scrolling in some cases, specifically in the sms app
Testing completed: just landed on m-c. fixes the bugs I was seeing.
Risk to taking this patch (and alternatives if risky): this should be pretty low risk, it moves overlay scrollbars up in the overall page stack by at most one slot if the item they are moving above contains the content the scrollbars are scrolling. it has a pretty limited effect, and should only improve things in those cases.
String or UUID changes made by this patch: none
Attachment #8381024 - Flags: approval-mozilla-b2g28?
Flags: needinfo?(tnikkel)
Attachment #8381024 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
You need to log in before you can comment on or make changes to this bug.