1096542 - Tab bar jitters when full

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Andrew Martin McDonough (:mmcdonough)]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20141110004002

Steps to reproduce:

Open enough tabs to fill the tab bar.
Open a new tab with the middle click.


Actual results:

The arrow to scroll through tabs very quickly appears and disappears.


Expected results:

The arrow to scroll through tabs should just stay there.

Comment 1

[:Gijs (out for now; he/him)]

Is this while using the devedition theme?

Comment 2

[Andrew Martin McDonough (:mmcdonough)]

Yes.

Comment 3

[Brian Grinstead [:bgrins]]

I'm not seeing this on 10.8.5.  Just to make sure, you are not seeing the problem with the default theme (with browser.devedition.theme.enabled = false)?

Comment 4

[Brian Grinstead [:bgrins]]

Attachment: tab-not-seeing-jitter.gif

I'm not seeing the jitter here (osx 10.10), unless if I'm missing something

Comment 5

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Attachment: attempt to repro

I tried to repro, not sure what I should be seeing. I don't see jitter.

Are you using a retina display?

Comment 6

[Andrew Martin McDonough (:mmcdonough)]

Here is what it looks like:

https://drive.google.com/file/d/0B-hkR9nzCDDKNm5wQU9DQjZqb2M/view

I noticed that mousing over the final tab stops the jitter.
If I have enough tabs open, it stops. I assume this is the point at which the tabs would stop shrinking in width and the scroll would be enabled.

I am using a Retina display.
This does not happen with the standard theme.

Comment 7

[Brian Grinstead [:bgrins]]

OK, I can see this too when I customize and add a toolbarbutton to the left of the 'add tab' button.  Not exact same STR, but if I open a bunch of tabs I saw the jitter then closed them right until the scroll arrows got disabled and started seeing it.

Comment 9

[Brian Grinstead [:bgrins]]

I believe this is caused by the overriding of tabCurveHalfWidth and tabWidth from Bug 1094782.  That was a fix for the scroll arrows remaining enabled even when they shouldn't.  Removing the whole block (including the .tabbrowser-arrowscrollbox > .arrowscrollbox-scrollbox that was there before that patch) solves both problems, but adds an extra padding to the sides of the tabs.

It's going to take some more investigation.  I'm guessing there is something else using the magic number 30px or 15px that isn't obvious, or I haven't overridden everything.

Comment 10

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Andrew Martin McDonough (:mmcdonough) from comment #6)
> Here is what it looks like:
> 
> https://drive.google.com/file/d/0B-hkR9nzCDDKNm5wQU9DQjZqb2M/view

WEEIIIIRRRDDD. And, thanks so much for providing that clip - it really helps. :)

Comment 11

[:Gijs (out for now; he/him)]

(In reply to Brian Grinstead [:bgrins] from comment #9)
> I believe this is caused by the overriding of tabCurveHalfWidth and tabWidth
> from Bug 1094782.  That was a fix for the scroll arrows remaining enabled
> even when they shouldn't.  Removing the whole block (including the
> .tabbrowser-arrowscrollbox > .arrowscrollbox-scrollbox that was there before
> that patch) solves both problems, but adds an extra padding to the sides of
> the tabs.
> 
> It's going to take some more investigation.  I'm guessing there is something
> else using the magic number 30px or 15px that isn't obvious, or I haven't
> overridden everything.

So, first off, that may be the case...

Second, as I noted on IRC at some point, we change the expected tab widths (these are prefs) for Australis. It's possible they may need adjusting for the devtools theme, too, although I'm not entirely sure why, as the total width should be the same - just no more overlap... :-\

Matt, can you help here?

Comment 12

[Matthew N. [:MattN]]

Can you still reproduce this jitter loop when the all-tabs dropdown isn't on the right of the Tabs Toolbar? That button is no longer customizable since Australis so you would have to restore defaults or edit the pref browser.uiCustomization.state by hand and restart.

Brian, when you reproduced this, was it just that you saw "The arrow to scroll through tabs very quickly appears and disappears." or did you see it get caught in a loop like the video shows?

A quick review of the deveditions styles seems to show all of the tabCurveHalfWidth and tabWidth being overridden. My other guess would have been tab separators causing the problem since hovering the last tab fixes the problem and the hover style differ depending on hover.

Comment 13

[Andrew Martin McDonough (:mmcdonough)]

I only ever had the all-tabs dropdown on the left. I tried it on both sides, it happens either way for me.
I'm not sure what you mean by it not being customizable, I can drag it around just fine in custromize-mode.

Comment 14

[:Gijs (out for now; he/him)]

(In reply to Andrew Martin McDonough (:mmcdonough) from comment #13)
> I only ever had the all-tabs dropdown on the left. I tried it on both sides,
> it happens either way for me.
> I'm not sure what you mean by it not being customizable, I can drag it
> around just fine in custromize-mode.

You're using an add-on that lets you do that, presumably? For me, it's not movable on latest nightly on OS X - and it shouldn't be...

Comment 15

[Andrew Martin McDonough (:mmcdonough)]

Ah, yes. I had Classic Theme Restorer installed.

Disabling it makes no difference, though.

Comment 16

[Sergey]

(In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #12)
> Can you still reproduce this jitter loop when the all-tabs dropdown isn't on
> the right of the Tabs Toolbar? That button is no longer customizable since
> Australis so you would have to restore defaults or edit the pref
> browser.uiCustomization.state by hand and restart.

I reproduced a similar jitter loop with devedition on empty profile in a few clicks. See bug 1096655 for steps to reproduce and attachment 8520281 [details] animation there.

Comment 17

[Brian Grinstead [:bgrins]]

(In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #12)
> Can you still reproduce this jitter loop when the all-tabs dropdown isn't on
> the right of the Tabs Toolbar? That button is no longer customizable since
> Australis so you would have to restore defaults or edit the pref
> browser.uiCustomization.state by hand and restart.
> 
> Brian, when you reproduced this, was it just that you saw "The arrow to
> scroll through tabs very quickly appears and disappears." or did you see it
> get caught in a loop like the video shows?
> 

I reproduced with the all tabs button in the default position (although I did have a button in the tab bar next to the new tab button.  But the reproduction in bug 1096655 doesn't have that special configuration.  I did something like this to reproduce:

Resize window to 975 px
Open 3 tabs
Open a 4th tab using middle click on a link from one of the others

Comment 18

[Brian Grinstead [:bgrins]]

(In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #12)
> A quick review of the deveditions styles seems to show all of the
> tabCurveHalfWidth and tabWidth being overridden. My other guess would have
> been tab separators causing the problem since hovering the last tab fixes
> the problem and the hover style differ depending on hover.

I don't think that we are changing the box model for the separators, what selector are you talking about?

The only other thing I see that looks suspicious is that we are setting the width/margin of #new-tab-button/.tabs-newtab-button (which I can't test at the moment in the middle of a build).

Where is the JS that is causing the tab resizing to happen?  I could try adding in some logging to try and track it down.

Comment 19

[Matthew N. [:MattN]]

(In reply to Brian Grinstead [:bgrins] from comment #18)
> (In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #12)
> > A quick review of the deveditions styles seems to show all of the
> > tabCurveHalfWidth and tabWidth being overridden. My other guess would have
> > been tab separators causing the problem since hovering the last tab fixes
> > the problem and the hover style differ depending on hover.
> 
> I don't think that we are changing the box model for the separators, what
> selector are you talking about?

I also didn't see the size of the separator changing for devedition but it's something I would guess without looking at the source.

> The only other thing I see that looks suspicious is that we are setting the
> width/margin of #new-tab-button/.tabs-newtab-button (which I can't test at
> the moment in the middle of a build).

That could be relevant.

> Where is the JS that is causing the tab resizing to happen?  I could try
> adding in some logging to try and track it down.

The tab resizing is just XUL flex IIRC. Overflow stuff is handle by tabbrowser.xml + scrollbox.xml + the place which fires the "overflow" event which I can't find.

Comment 20

[Brian Grinstead [:bgrins]]

(In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #19)
> > The only other thing I see that looks suspicious is that we are setting the
> > width/margin of #new-tab-button/.tabs-newtab-button (which I can't test at
> > the moment in the middle of a build).
> 
> That could be relevant.

No dice.  Same problem even with the changes to the new tab button removed.

Comment 21

[Brian Grinstead [:bgrins]]

Some more debugging:

* Still a problem with browser.tabs.animate set to false
* Doesn't happen with ctrl+t but does when middle clicking a link on the page

Comment 22

[Brian Grinstead [:bgrins]]

Attachment: jittering.mp4

Hovering over the last tab stops the jittering

Comment 23

[Brian Grinstead [:bgrins]]

I'm tempted to just remove the entire override since it is a much less bad problem - it just means there are 15 extra px to the left and right of the tab strip between the arrow scrollbars when overflowing

Comment 24

[Brian Grinstead [:bgrins]]

Commenting out the call to tabs.setAttribute("overflow", "true") in the overflow handler here gets rid of the problem: http://dxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#3573.  Looks like somehow the overflow handler is triggering an overflow event.

I think it's a due to _handleTabSelect calling this line:

this.mTabstrip.ensureElementIsVisible(this.selectedItem, aSmoothScroll);

Comment 25

[Brian Grinstead [:bgrins]]

OK, this is weird - if I change the overridden width to 1px / 2px the jitter still happens.  If I change that to 2px / 4px it isn't a problem (on osx).

I see the exact same problem in Australis (no devedition theme changes) if I change those overrides in tabs.inc.css to:

%define tabCurveWidth 2px
%define tabCurveHalfWidth 1px

Comment 26

[Brian Grinstead [:bgrins]]

Attachment: jitter.patch

This fixes the problem (although it's using a magic number that happens to work for me).  Given that this also happens with Australis when using these tab curve values, I'd like to just have a simple fix like this for an immediate and uplifted fix, and track down the underlying issue separately.  I still need to check other platforms - additionally, could anyone who can reproduce this please confirm that the patch fixes it for them?

I've got a try push (which will have binaries when it finishes building) here:  https://tbpl.mozilla.org/?tree=Try&rev=00f6ab844b64

Comment 27

[Matthew N. [:MattN]]

Comment on attachment 8520837 [details] [diff] [review]
jitter.patch

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

This is fine for the short term. I wouldn't be surprised if there is a truthiness check that is failing with the 0 somewhere.

Comment 28

[Matthew N. [:MattN]]

Maybe this is it here: http://hg.mozilla.org/mozilla-central/annotate/c60fc2c11c0e/browser/base/content/tabbrowser.xml#l3534

It seems like it wasn't made to handle a change from `n < 0` to `n >= 0` at runtime but the devedition stylesheet can get applied after the browser is running. This may not be the actual cause though.

Comment 29

[Brian Grinstead [:bgrins]]

The 'download states' error in the try push (https://tbpl.mozilla.org/?tree=Try&rev=00f6ab844b64) is unrelated - I get it on my local build even without the patch applied

Comment 30

[Brian Grinstead [:bgrins]]

 https://hg.mozilla.org/integration/fx-team/rev/d93e4bc5d08b

Comment 31

[Brian Grinstead [:bgrins]]

I haven't been able to confirm on linux / windows with or without the patch.  Sergey, Andrew, would you be able to check and see if this fixes it for you?

To test this, you can download and run the relevant binaries from https://tbpl.mozilla.org/?tree=Try&rev=00f6ab844b64.  Click the relevant green "B" next to your platform then click "go to build directory"

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d93e4bc5d08b

Comment 33

[Brian Grinstead [:bgrins]]

Comment on attachment 8520837 [details] [diff] [review]
jitter.patch

Approval Request Comment
[Feature/regressing bug #]: Devedition theme
[User impact if declined]: In certain cases with tab overflowing, the tabs start shaking out of control (see video), it also causes a CPU spike during this time.
[Describe test coverage new/current, TBPL]: None, CSS only
[Risks and why]: Low, CSS only for devedition theme
[String/UUID change made/needed]:

Comment 34

[Sergey]

(In reply to Brian Grinstead [:bgrins] from comment #31)
> I haven't been able to confirm on linux / windows with or without the patch.
> Sergey, Andrew, would you be able to check and see if this fixes it for you?
> 
> To test this, you can download and run the relevant binaries from
> https://tbpl.mozilla.org/?tree=Try&rev=00f6ab844b64.  Click the relevant
> green "B" next to your platform then click "go to build directory"

Can't check the bug with this build. It's branded "Firefox Nightly" and has no "Use Firefox Developer Edition Theme" button in about:customizing. I was only able to reproduce it with devedition theme active (see "Probably important things" in bug 1096655) which I can't turn on in this build.

Comment 35

[Matthew N. [:MattN]]

Sergey, you can change the pref browser.devedition.theme.enabled to true in that build and restart.

Comment 36

[Sergey]

(In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #35)
> Sergey, you can change the pref browser.devedition.theme.enabled to true in
> that build and restart.

With browser.devedition.theme.enabled set to true I still see a light theme, not a dark one as in devedition. The button is called "Use Nightly Theme" (browser.devedition.theme.showCustomizeButton=true). The bug does not happen with that "Nightly Theme".

To make sure I also downloaded https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-11-10-03-02-04-mozilla-central/firefox-36.0a1.en-US.linux-i686.tar.bz2 and set browser.devedition.theme.enabled and browser.devedition.theme.showCustomizeButton to true. And the bug does not happen there either.

I guess it does not happen on Nightly, just on aurora/devedition theme.

Comment 37

[Matthew N. [:MattN]]

Then you also need to switch to the dark devtools theme through the UI or set the pref devtools.theme to "dark"

Comment 38

[Brian Grinstead [:bgrins]]

 > With browser.devedition.theme.enabled set to true I still see a light theme,
> not a dark one as in devedition. The button is called "Use Nightly Theme"
> (browser.devedition.theme.showCustomizeButton=true). The bug does not happen
> with that "Nightly Theme".

You are doing the right thing, but the strings there are confusing - sorry!  Basically, the button is just hardcoded to "Use [product name] theme" in the assumption that it will only show up in dev edition.

If you are not seeing the problem here, then I'd say it's verified to be fixed on your system (the light/dark devedition theme will have no bearing on this particular issue).  Thanks for following up - this should be fixing on devedition once it gets approval and gets uplifted.

Comment 40

[Sergey]

(In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #37)
> Then you also need to switch to the dark devtools theme through the UI or
> set the pref devtools.theme to "dark"

That makes the difference! The bug did not happen with devtools.theme="light" for some Nightly.

(In reply to Brian Grinstead [:bgrins] from comment #38)
> If you are not seeing the problem here, then I'd say it's verified to be
> fixed on your system (the light/dark devedition theme will have no bearing
> on this particular issue).  Thanks for following up - this should be fixing
> on devedition once it gets approval and gets uplifted.

I guess so. Tested https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/ :
2014-11-10-03-02-04-mozilla-central - Nightly 2014-11-10 - bug 1096655 for "dark", but not "light"
2014-11-11-03-02-03-mozilla-central - Nightly 2014-11-11 - bug 1096655 for both "dark" and "light"
2014-11-12-03-02-02-mozilla-central - Nightly 2014-11-12 - no bug for both "dark" and "light"

about:config changes used for test:
  browser.devedition.theme.enabled = true
  browser.devedition.theme.showCustomizeButton = true
  devtools.theme = dark

Thank you!

Comment 41

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/621176ae1dcf

Comment 43

[pratyasmitamishra]

Bugday-20141218: The arrow to scroll behaves as expected.