1094830 - Open a new tab button shakes after it reaches the right edge of Firefox

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Bogdan Maris, Desktop Test Engineering]

Attachment: Screencast showing the issue

STR:
1. Open Firefox Developer Edition.
2. Open new tabs until the '+' button (open a new tab button) reaches the right edge.
3. Click on '+' button to add more tabs.

Expected result: The button does not shake.

Actual result: Button shakes if it`s clicked after it reaches the right edge of FF until navigation arrow buttons appear, after that happens it returns to normal.

I also noticed that on Ubuntu the '+' button is colored blue, is this intended?

Notes:
1. This issue reproduces on Windows and Ubuntu only.
2. Not reproducible on Mac OS X.
3. Screencast showing the issue attached.

Comment 1

[Brian Grinstead [:bgrins]]

> I also noticed that on Ubuntu the '+' button is colored blue, is this
> intended?

That's what happens even on lightweight themes on linux.  Take a look at bug 940326 for more information

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #0)
> Created attachment 8518167 [details]
> Screencast showing the issue
> 
> STR:
> 1. Open Firefox Developer Edition.
> 2. Open new tabs until the '+' button (open a new tab button) reaches the
> right edge.
> 3. Click on '+' button to add more tabs.
> 
> Expected result: The button does not shake.
> 
> Actual result: Button shakes if it`s clicked after it reaches the right edge
> of FF until navigation arrow buttons appear, after that happens it returns
> to normal.
> 

Thanks for the report, are you talking about the icon subtly moving location during the click, or the button itself disappearing from under the mouse once the 'all tabs' button shows up?

Comment 3

[Tim Nguyen :ntim]

I can reproduce this, it must be because of the selectors we use for the tabs.

Comment 4

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #3)
> I can reproduce this, it must be because of the selectors we use for the
> tabs.

*tab separators.

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Tim Nguyen [:ntim] from comment #4)
> (In reply to Tim Nguyen [:ntim] from comment #3)
> > I can reproduce this, it must be because of the selectors we use for the
> > tabs.
> 
> *tab separators.

I doubt that it is the tab separators, since that is now just changing the background-image and no longer changing widths/margins like the original version did.

Comment 6

[Bogdan Maris, Desktop Test Engineering]

(In reply to Brian Grinstead [:bgrins] from comment #2)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #0)
> > Created attachment 8518167 [details]
> > Screencast showing the issue
> > 
> > STR:
> > 1. Open Firefox Developer Edition.
> > 2. Open new tabs until the '+' button (open a new tab button) reaches the
> > right edge.
> > 3. Click on '+' button to add more tabs.
> > 
> > Expected result: The button does not shake.
> > 
> > Actual result: Button shakes if it`s clicked after it reaches the right edge
> > of FF until navigation arrow buttons appear, after that happens it returns
> > to normal.
> > 
> 
> Thanks for the report, are you talking about the icon subtly moving location
> during the click, or the button itself disappearing from under the mouse
> once the 'all tabs' button shows up?

I`m talking about the button moving during the click.

Comment 7

[Brian Grinstead [:bgrins]]

I believe I've tracked this down when looking at 1093820.  Basically, there is a custom width on .tabs-newtab-button and #new-tab-button.  I think we only want that set on .tabs-newtab-button (at least on Windows).

Comment 8

[Brian Grinstead [:bgrins]]

Attachment: tab-shake.patch

There were a few things going on here:
1) There was a margin: 0 that was meant to override the 1px margin-bottom from @tabToolbarNavbarOverlap@.  The problem is it wasn't applied to all buttons that have that overlap.  I've moved that into it's own more specific and limited rule.  (I know this still isn't totally ideal, but until bug 1091001 I think it's an improvement)
2) There was a width of 35px set to #TabsToolbar > #new-tab-button and .tabs-newtab-button. First of all, that should only have been set to .tabs-newtab-button (since the #new-tab-button is skinnier when overflowing in Windows).  Secondly, that width should have been 36 (66px from tabs.inc - 30px for the tab curves).
3)  There was an !important on the background-image rule, but as best as I can tell it is because of :hover rules like this: http://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/tabs.inc.css#269.  By moving it into the :hover rule I got rid of the !important.

I've checked on osx, windows 7 aero / classic / basic, and ubuntu and there is no more shaking in them.

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 8521086 [details] [diff] [review]
tab-shake.patch

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

::: browser/themes/shared/devedition.inc.css
@@ +306,2 @@
>  .tabs-newtab-button {
> +  width: 36px;

Would you mind converting https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/tabs.inc.css?rev=3adeab7e472d#319 to a calc() using @tabCurveWidth@ so we would catch this in the future?

Comment 10

[Matthew N. [:MattN]]

Does this mean we can remove the other workaround for jittering that landed yesterday?

Comment 11

[Brian Grinstead [:bgrins]]

Attachment: tab-shake-r=MattN.patch

Using calc() in tabs.inc.css to make sure we don't miss this in the future

Comment 12

[Brian Grinstead [:bgrins]]

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

Comment 13

[Brian Grinstead [:bgrins]]

(In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #10)
> Does this mean we can remove the other workaround for jittering that landed
> yesterday?

I think that was still an issue even with this fix, but I'll double check.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/882f3497f8aa

Comment 15

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #13)
> (In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #10)
> > Does this mean we can remove the other workaround for jittering that landed
> > yesterday?
> 
> I think that was still an issue even with this fix, but I'll double check.

Nope, sadly still jittering if I set the width back to 0px even with this fix.  It's unfortunate because now I realized that the 2px fix makes the selected tab look a little weird in windows / linux (it's not directly lined up with the side of the browser window anymore).

Comment 16

[Matthew N. [:MattN]]

Sorry, I thought you realized that was a side-effect.

Comment 17

[pratyasmitamishra]

Bugday-20141218 :Button still shakes if it`s clicked after it reaches the right edge of FF until navigation arrow buttons appear, after that happens it returns to normal.

Comment 18

[pratyasmitamishra]

Bugday-20141218 : The button does not shake and behaves as expected.

Comment 19

[pratyasmitamishra]

Bugday-20141218 : The button does not shake and behaves as expected.

Comment 20

[pratyasmitamishra]

Bugday-20141218 : The button does not shake and behaves as expected.