A scrollbar appear at the bottom of the menu panel when entering or exiting customization mode

VERIFIED FIXED in Firefox 36

Status

()

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: ge3k0s, Assigned: mats)

Tracking

({regression})

Trunk
Firefox 38
All
Windows 7
regression
Points:
---
Bug Flags:
firefox-backlog +
in-testsuite -
qe-verify +

Firefox Tracking Flags

(firefox33 unaffected, firefox34+ wontfix, firefox35+ wontfix, firefox36+ verified, firefox37+ verified, firefox38 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Created attachment 8514363 [details]
Scrollbar during animation at the bottom of the panel

At least on Windows a scroll bar briefly flickers at the bottom of the menu panel when entering or exiting customization mode. I don't have a regression range but I would say about a few weeks back.
(Reporter)

Updated

4 years ago
Keywords: regression, regressionwindow-wanted

Comment 1

4 years ago
(In reply to Guillaume C. [:ge3k0s] from comment #0)
> Created attachment 8514363 [details]
> Scrollbar during animation at the bottom of the panel
> 
> At least on Windows a scroll bar briefly flickers at the bottom of the menu
> panel when entering or exiting customization mode. I don't have a regression
> range but I would say about a few weeks back.

A few like multiple? A regression window here would be super useful. I'd be worried about this being caused by bug 1074520 if it's more recent than 2 weeks ago...

Comment 2

4 years ago
(and I mean, is this on nightly? anywhere else you see this?)
(Reporter)

Comment 3

4 years ago
(In reply to :Gijs Kruitbosch from comment #1)
> (In reply to Guillaume C. [:ge3k0s] from comment #0)
> > Created attachment 8514363 [details]
> > Scrollbar during animation at the bottom of the panel
> > 
> > At least on Windows a scroll bar briefly flickers at the bottom of the menu
> > panel when entering or exiting customization mode. I don't have a regression
> > range but I would say about a few weeks back.
> 
> A few like multiple? A regression window here would be super useful. I'd be
> worried about this being caused by bug 1074520 if it's more recent than 2
> weeks ago...

No I really think it's older than that. I may have the time to find a regression range in a few days. I see this on Nightly. Will try Aurora now.
(Reporter)

Comment 4

4 years ago
Reproducible on Nightly, Aurora, Beta but not on release. So you may be right about the concerned bug.
(Reporter)

Comment 5

4 years ago
(In reply to Guillaume C. [:ge3k0s] from comment #4)
> Reproducible on Nightly, Aurora, Beta but not on release. So you may be
> right about the concerned bug.

No the issue is at least one month old.

Comment 6

4 years ago
(In reply to Guillaume C. [:ge3k0s] from comment #5)
> (In reply to Guillaume C. [:ge3k0s] from comment #4)
> > Reproducible on Nightly, Aurora, Beta but not on release. So you may be
> > right about the concerned bug.
> 
> No the issue is at least one month old.

but it was uplifted, then, if beta is affected? :-(
(Reporter)

Comment 7

4 years ago
(In reply to :Gijs Kruitbosch from comment #6)
> (In reply to Guillaume C. [:ge3k0s] from comment #5)
> > (In reply to Guillaume C. [:ge3k0s] from comment #4)
> > > Reproducible on Nightly, Aurora, Beta but not on release. So you may be
> > > right about the concerned bug.
> > 
> > No the issue is at least one month old.
> 
> but it was uplifted, then, if beta is affected? :-(

Sorry I wasn't clear it's at least a month old on Nightly.

Comment 8

4 years ago
This regressed end of August.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: qe-verify+
Flags: in-testsuite-
Flags: firefox-backlog+

Comment 10

4 years ago
(In reply to :Gijs Kruitbosch from comment #9)
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=eed9fe35a00d&tochange=1db35d2c9a2f

I... have no idea what caused this, actually. Mike/Jared?
Flags: needinfo?(mconley)
Flags: needinfo?(jaws)
Wow, nothing at all stands out in that range. :/ I think we need to bisect.
Flags: needinfo?(mconley)
My guesses:

9c0d8ef32b3e	Markus Stange — Bug 1021564 - Invalidate filtered frames when they move in certain ways. r=roc
4911e706d559	Botond Ballo — Bug 1058884 - Update drawing of borders to account for multi-layer-apz. r=kats,BenWa
bdcf3a2da55c	Neil Rashbrook — Bug 1054289 Scroll to the current ref, not the original one r=smaug

But out of those I think it may be the first one. Markus, could this be fallout from your change?
Flags: needinfo?(jaws) → needinfo?(mstange)

Comment 13

4 years ago
[Tracking Requested - why for this release]: perf/visual regression.

(Markus, if you have time to look at this, that'd still be much appreciated)
status-firefox33: --- → unaffected
status-firefox34: --- → affected
status-firefox35: --- → affected
status-firefox36: --- → affected
tracking-firefox34: --- → ?
Flags: needinfo?(gijskruitbosch+bugs)

Comment 14

4 years ago
local testing says the regression range is:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5fd6428ae179&tochange=983cf2175495

trying to bisect further at the minute... but this excludes Markus' change.
Flags: needinfo?(mstange)
Flags: needinfo?(gijskruitbosch+bugs)

Comment 15

4 years ago
(In reply to :Gijs Kruitbosch from comment #14)
> local testing says the regression range is:
> 
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=5fd6428ae179&tochange=983cf2175495
> 
> trying to bisect further at the minute... but this excludes Markus' change.

I'm no longer sure about this. :-\

Comment 16

4 years ago
So, believe it or not... it seems this was regressed by bug 1060165, specifically, by https://hg.mozilla.org/mozilla-central/rev/2123851421e6 . Verified by building:

https://hg.mozilla.org/integration/fx-team/rev/c98e45bed5e0 + ffxbld patches: works

https://hg.mozilla.org/integration/fx-team/rev/c98e45bed5e0 + ffxbld patches + 2123851421e6: broken

Nical/David: any ideas? My only hunch here (as someone who totally doesn't know graphics/layout) is that this:

https://hg.mozilla.org/mozilla-central/rev/2123851421e6#l15.2

has unintended consequences.
Blocks: 1060165
Flags: needinfo?(nical.bugzilla)
Flags: needinfo?(dzbarsky)

Comment 17

4 years ago
(In reply to :Gijs Kruitbosch from comment #16)
> Nical/David: any ideas? My only hunch here (as someone who totally doesn't
> know graphics/layout) is that this:
> 
> https://hg.mozilla.org/mozilla-central/rev/2123851421e6#l15.2
> 
> has unintended consequences.

This hunch is wrong, please disregard. :-)
I'm tracking this visual regression but unless we have a safe fix by Wed, Nov 12, we're going to ship with this bug as it doesn't look critical enough to hold the release.
tracking-firefox34: ? → +
tracking-firefox35: --- → +
tracking-firefox36: --- → +
This is in the realm of layout which I don't know as well as I would like to. I don't know what could cause this.
Flags: needinfo?(nical.bugzilla)
I'm marking 34 as wontfix. This is still a visual regression that we should fix.

Jet - FYI, Nical says that this is layout related. Can you please help find an owner to investigate further?
status-firefox34: affected → wontfix
Flags: needinfo?(bugs)
(In reply to :Gijs Kruitbosch from comment #17)
> (In reply to :Gijs Kruitbosch from comment #16)
> > Nical/David: any ideas? My only hunch here (as someone who totally doesn't
> > know graphics/layout) is that this:
> > 
> > https://hg.mozilla.org/mozilla-central/rev/2123851421e6#l15.2
> > 
> > has unintended consequences.
> 
> This hunch is wrong, please disregard. :-)

Is the regression window still correct?
Flags: needinfo?(gijskruitbosch+bugs)

Comment 22

4 years ago
(In reply to Jet Villegas (:jet) from comment #21)
> (In reply to :Gijs Kruitbosch from comment #17)
> > (In reply to :Gijs Kruitbosch from comment #16)
> > > Nical/David: any ideas? My only hunch here (as someone who totally doesn't
> > > know graphics/layout) is that this:
> > > 
> > > https://hg.mozilla.org/mozilla-central/rev/2123851421e6#l15.2
> > > 
> > > has unintended consequences.
> > 
> > This hunch is wrong, please disregard. :-)
> 
> Is the regression window still correct?

Yes! I'm very sure about the regression window, just clueless as to how that caused this issue...
Flags: needinfo?(gijskruitbosch+bugs)
We're now too late for speculative fixes on 35, tracking for 36/37 still since we have identified a regression window - we need an assignee here and for either a backout option or a forward fix to deal with this visual regression.
status-firefox35: affected → wontfix
status-firefox37: --- → affected
tracking-firefox37: --- → +
Jet, could you find someone to help us on this? Thanks
(Assignee)

Comment 25

4 years ago
gfxPoint3D was using gfxFloat (double) internally.  Point3D is using Float (float),
so we lost precision in some operations when changing gfxPoint3D => Point3D.
I'm a bit surprised the conversion didn't cause more bugs, but I guess gfxPoint3D
wasn't used much.  Anyway, please test these builds when they are ready:
https://tbpl.mozilla.org/?tree=Try&rev=0150ce95a7af
Flags: needinfo?(gijskruitbosch+bugs)

Comment 26

4 years ago
(In reply to Mats Palmgren (:mats) from comment #25)
> gfxPoint3D was using gfxFloat (double) internally.  Point3D is using Float
> (float),
> so we lost precision in some operations when changing gfxPoint3D => Point3D.
> I'm a bit surprised the conversion didn't cause more bugs, but I guess
> gfxPoint3D
> wasn't used much.  Anyway, please test these builds when they are ready:
> https://tbpl.mozilla.org/?tree=Try&rev=0150ce95a7af

Yay! That does indeed fix it for me.
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 27

4 years ago
Created attachment 8551342 [details] [diff] [review]
fix

This just inlines Transform3D and use local gfxFloats (double) instead
of passing point.x/y via a Point3D (float) to avoid losing precision
(also skipping the z component which we don't need).  I intend to 
uplift this to Aurora/Beta too.
Assignee: nobody → mats
Attachment #8551342 - Flags: review?(matt.woodrow)
Attachment #8551342 - Flags: review?(matt.woodrow) → review+
(Assignee)

Comment 29

4 years ago
Comment on attachment 8551342 [details] [diff] [review]
fix

Approval Request Comment
[Feature/regressing bug #]: bug 1060165
[User impact if declined]: (rare) layout errors (triggering a scrollbar when not needed etc) when using transforms.
[Describe test coverage new/current, TreeHerder]: green on Try
[Risks and why]: low risk; the patch basically reverts gfx3DMatrix::Transform(const gfxPoint&) to its pre-1060165 behaviour.
[String/UUID change made/needed]: none
Attachment #8551342 - Flags: approval-mozilla-beta?
Attachment #8551342 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 30

4 years ago
FWIW, it seems like it improves performance as well:

Improvement: Mozilla-Inbound-Non-PGO - Customization Animation Tests - WINNT 6.2 x64 - 3.68% decrease
-----------------------------------------------------------------------------------------------------
    Previous: avg 29.022 stddev 0.337 of 12 runs up to revision 973ede87dcdd
    New     : avg 27.954 stddev 0.334 of 12 runs since revision 538af3cc79b2
    Change  : -1.067 (3.68% / z=3.167)
    Graph   : http://mzl.la/1Cffu6O
https://hg.mozilla.org/mozilla-central/rev/538af3cc79b2
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38

Updated

4 years ago
Iteration: --- → 38.1 - 26 Jan
Flags: needinfo?(dzbarsky)
status-firefox38: --- → fixed
Attachment #8551342 - Flags: approval-mozilla-beta?
Attachment #8551342 - Flags: approval-mozilla-beta+
Attachment #8551342 - Flags: approval-mozilla-aurora?
Attachment #8551342 - Flags: approval-mozilla-aurora+
Keywords: qawanted
QA Contact: cornel.ionce
Flags: needinfo?(bugs)
(In reply to Sylvestre Ledru [:sylvestre] from comment #24)
> Jet, could you find someone to help us on this? Thanks

Mats! \o/
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0

Confirming the fix on latest Nightly, build ID: 20150121030203, the scrollbar is no longer displayed when entering/exiting Customize Mode.
Marking verified as the target Milestone is reached.

Will verify tomorrow on latest Aurora and Beta.
Status: RESOLVED → VERIFIED
status-firefox38: fixed → verified
Keywords: qawanted
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:37.0) Gecko/20100101 Firefox/37.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0

Verified fixed on latest Aurora (build ID: 20150125004007) and on Firefox 36 beta 3 (build ID: 20150122214638).
status-firefox36: fixed → verified
status-firefox37: fixed → verified
You need to log in before you can comment on or make changes to this bug.