1121723 - DevEdition theme: tab bar jittering when in customize mode that is not the last tab

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Brian Grinstead [:bgrins]]

I've been able to reproduce on OSX.

STR:

Open up enough tabs to overflow
Open customize mode
Press cmd+t to open a new tab so the customize tab isn't the last
Switch to customize tab

Jittering starts while in customize mode and continues until exiting.

Comment 1

[Brian Grinstead [:bgrins]]

I've noticed that removing the "#TabsToolbar[currentset]:not([currentset*="tabbrowser-tabs,new-tab-button"])" portion of the selector here seems to fix the problem: https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/tabs.inc.css#327.

That selector is intended to handle the case with a separated new tab button, but doesn't match in customize mode.

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: jittering-customize.patch

A different approach than outlined in Comment 1 - this just switches the separators to only be 1px wide and margin-start -1px so that we don't have the issue with extending the width of the final tab.  I can't find anything specifically expecting this to be 3px in tabbrowser or anywhere else, but let me know if you think changing this width is likely to be a problem.

This also lets us remove the ugly workaround from Bug 1111091 dealing with a separated new tab button.

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8556768 [details] [diff] [review]
jittering-customize.patch

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

Outside dev-edition the image is 3px wide but other than that I can't think of other reliance on 3px.

I think the changes to devedition.inc.css are fine but I thought that the code you're deleting in tabs.inc.css could be useful outside of devedition? Am I remembering incorrectly? I don't think it was a problem without extensions though and we lived with that for months until you added the hunk recently so I think it's not a big deal either way.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Matthew N. [:MattN] from comment #3)
> I think the changes to devedition.inc.css are fine but I thought that the
> code you're deleting in tabs.inc.css could be useful outside of devedition?
> Am I remembering incorrectly? I don't think it was a problem without
> extensions though and we lived with that for months until you added the hunk
> recently so I think it's not a big deal either way.

I believe this was only an issue with tabCurveWidth < 2px - I've never seen the jitter without the devedition theme applied.

When debugging it I set @tabCurveWidth@/@tabCurveHalfWidth@ to 0px on the default theme and was able to reproduce and narrow down the problem to this instead of something else within the devedition theme, so maybe that is what you are thinking of.

Comment 5

[Matthew N. [:MattN]]

Perhaps. This is something add-ons could do though.

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Matthew N. [:MattN] from comment #5)
> Perhaps. This is something add-ons could do though.

Are they able to change the preprocessor values?

Comment 7

[Matthew N. [:MattN]]

No, but they could override the values like dev edition does.

Comment 8

[Brian Grinstead [:bgrins]]

Attachment: pinned-overflow-border.png

Turns out this causes the first separator to be invisible in the case of overflowing tabs with at least one pinned tab (see screenshot).  It seems that switching the -moz-margin-start and -moz-margin end values fixes this problem, but I'm not sure if this is going to cause different issues.

Comment 9

[Brian Grinstead [:bgrins]]

Attachment: jittering-customize2.patch

This fixes the problem in Comment 8 and also fixes the original issue, but I'm not sure if it is the right approach.  I'm thinking the difference is just that the separator shows up on the right side of the edge of the tab instead of the left.

Comment 10

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #9)
> Created attachment 8558209 [details] [diff] [review]
> jittering-customize2.patch
> 
> This fixes the problem in Comment 8 and also fixes the original issue, but
> I'm not sure if it is the right approach.  I'm thinking the difference is
> just that the separator shows up on the right side of the edge of the tab
> instead of the left.

I guess an alternative would be to use `moz-margin-start: -1px; -moz-margin-end: 0;` for the last tab (to make sure that the width of the last tab isn't extended) and then `moz-margin-start: 0; -moz-margin-end: -1px;` for all the others (so that the first separator issue in Comment 8 is resolved).

Comment 11

[Brian Grinstead [:bgrins]]

Attachment: jittering-customize.patch

Uses approach from Comment 10

Comment 12

[Brian Grinstead [:bgrins]]

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

Comment 13

[Phil Ringnalda (:philor)]

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