Closed Bug 1089380 Opened 5 years ago Closed 5 years ago

Scrollbar disappears in sidebar with position:fixed

Categories

(Core :: Graphics: Layers, defect)

34 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla36
Tracking Status
firefox33 --- unaffected
firefox34 + verified
firefox35 + verified
firefox36 + verified
firefox-esr31 --- unaffected

People

(Reporter: quicksaver, Assigned: BenWa)

References

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

Attached file OmniSIdebar 1.5a1
Last good build: firefox-34.0a1-2014-08-28
First bad build: firefox-34.0a1-2014-08-29

STR:

1. Install OmniSidebar xpi package attached
(1.1. Bookmarks sidebar should open, if not, open the sidebar, by clicking the window margin or hitting Ctrl+B)
2. Open add-ons manager in the sidebar (click the sidebar title for a dropdown of options to open)
3. Make sure you're in a pane that you can scroll
4. Undock the sidebar (button next to the sidebar close button), so it appears over the webpage instead of next to it
5. Un-maximize the window (normal mode)
6. Try scrolling the add-ons list

For example, if I try scrolling my add-ons list, the scrollbar's thumb (is that what it's called? the thing you drag...) will disappear. This issue does not occur if the window is maximized, which makes me think this may not be something I can fix within the add-on. Since I can reproduce this in the latest beta, this is kind of urgent!

It has always worked correctly in previous versions of firefox, including the current release (33). The issue also doesn't happen if the sidebar is docked (not position:fixed), or if I disable pref layers.offmainthreadcomposition.enabled (requires restart).

[Tracking Requested - why for this release]: If my suspicion that I can't fix this within the add-on is correct, the add-on will break in Firefox 34 and there's nothing I can do about it. (I hope I'm using the tracking flag correctly for this, please let me know if not.)
See Also: 1062100
Sounds like bug 1074165, a regression from bug 957445 which landed around that time.
Please try the latest FF34 build and let us know if it still occurs:
https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-win32-debug/1414335250/
Actually, after reading your description more carefully, it's probably something else
that regressed it.  Anyway, still worth checking the build above to see if it's been
fixed since the latest released beta.
Flags: needinfo?(quicksaver)
(In reply to Luís Miguel [:Quicksaver] from comment #0)
> Last good build: firefox-34.0a1-2014-08-28
> First bad build: firefox-34.0a1-2014-08-29

Based on that I'm guessing this is the regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3be45b58fc47&tochange=d697d649c765

Luís, can you verify the revision IDs in the URL above are the ones in the builds
you tested?  (load about:buildconfig in each build to see its rev)  Thanks.
(In reply to Mats Palmgren (:mats) from comment #2)
> Sounds like bug 1074165, a regression from bug 957445 which landed around
> that time.

From those bugs descriptions I also agree with your comment 3, I don't think that is the cause.

> Please try the latest FF34 build and let us know if it still occurs:
> https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-
> beta-win32-debug/1414335250/

Issue happens as I first described with that build (so does bug 1089382 btw).

(In reply to Mats Palmgren (:mats) from comment #4)
> Based on that I'm guessing this is the regression range:
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=3be45b58fc47&tochange=d697d649c765
> 
> Luís, can you verify the revision IDs in the URL above are the ones in the
> builds
> you tested?  (load about:buildconfig in each build to see its rev)  Thanks.

That's it. (How did I not know about about:buildconfig... You just revolutionized my debugging workflow!)
Flags: needinfo?(quicksaver)
(In reply to Mats Palmgren (:mats) from comment #6)
> In that regression range my first guess would be bug 1055760.

I had seen that, but since it is marked for platform All::Gonk (Firefox OS) I assumed it was just for FxOS. I take it that was a bad assumption?
(In reply to Luís Miguel [:Quicksaver] from comment #7)
> (In reply to Mats Palmgren (:mats) from comment #6)
> > In that regression range my first guess would be bug 1055760.
> 
> I had seen that, but since it is marked for platform All::Gonk (Firefox OS)
> I assumed it was just for FxOS. I take it that was a bad assumption?

The changes in bug 1055760 should only affect APZ platforms (currently B2G and Metro). I'd be quite surprised if those changes caused this regression.
Perhaps check bug 1010584? A bug in that culling code has the capacity to cause something to not be rendered.
That's another bug that I had seen, but it is marked for Mac OS X, so I didn't think it would be relevant in Windows. (The fact that everything is displayed correctly when the window is maximized, and not when it's not, made me think the issue might be Windows-specific.)

Unfortunately I don't understand enough of that code to evaluate it myself. Benoit, you worked on bug 1010584, do you think it could have caused this?
Flags: needinfo?(bgirard)
(In reply to Luís Miguel [:Quicksaver] from comment #10)
> That's another bug that I had seen, but it is marked for Mac OS X, so I
> didn't think it would be relevant in Windows. 

In this case, the platform field was set incorrectly, probably thanks to Bugzilla's "helpful" feature where it pre-sets it to the platform of the computer you're filing the bug from :)

The culling bug actually affects all OMTC platforms.

> (The fact that everything is
> displayed correctly when the window is maximized, and not when it's not,
> made me think the issue might be Windows-specific.)

It's possible; bug 1010584 it's just a guess.
BTW, does anyone know if there's anywhere I can get tinderbox builds older than last month? Maybe they could help narrow it down.
Kats, that was perfect, thanks so much!

Revised regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=70c1c94cfaec&tochange=2a15dc07ddaa

Bug 1010584 is in there. Also please see bug 1089381 which has that same regression range.
Depends on: 1010584
Flags: needinfo?(bgirard) → needinfo?(matt.woodrow)
Just confirmed that bug 1010584 caused this: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b28968baf343&tochange=e4aa884c7624

Unless you think bug 1058919 might have caused it? Although if I understand (enough of) it correctly, it shouldn't be that.

(I'm sorry about all the bugspam, still learning how to properly use tinderbox/inbound builds.)
Component: Layout → Graphics: Layers
Yep, sorry, I should have been more specific about that.
I've been trying to come up with a more reduced test-case, but for example just setting position:fixed to #sidebar-box in a clean profile doesn't seem to trigger this bug by itself. Apparently it's the whole combination of CSS properties that the add-on is setting that causes the bug, and there are quite a few of them, I haven't been able to find out which ones are actually relevant...

Please let me know if you do need a more reduced test-case and I will try again.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
Actually, this is a regression from BenWa's changes.

BenWa, the problem here is that the visible region of the parent in ClipRectInLayersCoordinates starts at -600, so we offset our clip by that and then pass it into DrawQuad.

I think we should probably just back that patch out entirely since we have alternative culling now, and I'm pretty convinced that the change to ClipRectInLayersCoordinates is incorrect.
Assignee: matt.woodrow → bgirard
I can reproduce a much worse bug by following the STR and resizing the window.
Is that OS X? I saw something similar this morning (testing for another bug) and was going to test further a bit later today and comment here about it, but it wasn't as bad as that.
Yes 10.9, when the apple GL drivers aren't happy they sometimes explode like that.
After removing my changes to that function it basically did nothing. Removing it now.

This fixes the issue for me.
Attachment #8515238 - Flags: review?(matt.woodrow)
Attachment #8515238 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/5ccd5638a7b2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
See Also: 1089381
Duplicate of this bug: 1089381
Blocks: 1089382
See Also: 1089382
BenWa - Looks like this is all code removal. What do you think about uplifting to beta, which is marked as affected?
Flags: needinfo?(bgirard)
Comment on attachment 8516883 [details] [diff] [review]
Remove ClipRectInLayersCoordinates + remove redundant culling

Approval Request Comment
[Feature/regressing bug #]: bug 1010584
[User impact if declined]: Graphical glitches form excessive culling as note in this bug (screnshot)
[Describe test coverage new/current, TBPL]: On central, this is covered extensively by reftest.
[Risks and why]: These a lot of test coverage for this but the risk is still medium. What we don't cover could still show a severe bug similar to this one.
[String/UUID change made/needed]: none
Flags: needinfo?(bgirard)
Attachment #8516883 - Flags: approval-mozilla-beta?
Attachment #8516883 - Flags: approval-mozilla-aurora?
Comment on attachment 8516883 [details] [diff] [review]
Remove ClipRectInLayersCoordinates + remove redundant culling

Beta+
Aurora+
Attachment #8516883 - Flags: approval-mozilla-beta?
Attachment #8516883 - Flags: approval-mozilla-beta+
Attachment #8516883 - Flags: approval-mozilla-aurora?
Attachment #8516883 - Flags: approval-mozilla-aurora+
QAWanted to see about doing some targeted testing on the affected elements for beta8.
Keywords: qawanted
Flags: qe-verify+
Following STR in comment 0, verified fixed on latest beta, aurora (developer ed.), and nightly.
Thanks Luis Miguel. I was able to reproduce this bug on Nightly 36.0a1 (2014-10-26) using Windows 7 x64 and I also verified fixed on Latest Nightly 36.0a1 (2014-11-16) using Windows 7 x64. 

Based on the above, I am making this as Verified Fixed.
Per Comment 36 & Comment 38 & Comment 39,clear "qawanted".
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.