865698 - Avoid using generated content: "" as much as possible

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[:Gijs (he/him)]

Related to bug 852420, we should just try to avoid using content: "" where we can, especially because we can stick the images in the actual content: rather than as a background image.

Comment 1

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #0)
> especially because we can stick the images in the actual content:
> rather than as a background image.

Does this actually avoid bug 852420? I wouldn't expect it to. Also, with the image injected as content, I don't think it would behave like you'd expect when using -moz-box, e.g. it wouldn't stretch.

Comment 2

[:Gijs (he/him)]

Attachment: Patch

This was actually surprisingly little work (less than border-image!) and this patch by itself seems to work on OS X, and is net code + selector removal! :-)

I've started a try run to look at how this looks on Windows and Linux: https://tbpl.mozilla.org/?tree=Try&rev=61cc4e06702a

Comment 3

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #2)
> I've started a try run to look at how this looks on Windows and Linux:
> https://tbpl.mozilla.org/?tree=Try&rev=61cc4e06702a

AFAICT, this looks fine Windows and Linux, too.

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 741898 [details] [diff] [review]
Patch

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

It would be good to make sure this will align with what's needed for bug 850918 so we don't just end up reverting this. Would you mind taking that bug and getting review on it before landing this?

::: browser/themes/shared/tabs.inc.css
@@ +87,5 @@
>  .tab-background-start[selected=true]::after,
>  .tab-background-end[selected=true]::after {
>    /* position ::after on top of its parent */
>    -moz-margin-start: -@tabCurveWidth@;
> +  margin-bottom: -1px;

This is going to have to change (to -2px) in bug 858089 but I guess we can do this in the meantime.

@@ +192,5 @@
>  .tabbrowser-tab:not([selected]):not([afterselected-visible]):not([afterhovered]):not([first-visible-tab]):not(:hover)::before,
>  #tabbrowser-tabs:not([overflow]) > .tabbrowser-tab[last-visible-tab]:not([selected]):not([beforehovered]):not(:hover)::after {
>    -moz-margin-start: -1.5px;
>    -moz-margin-end: -1.5px;
> +  content: url("chrome://browser/skin/tabbrowser/tab-separator.png");

Nit: Would you mind removing the quotes while you're touch this line so it's consistent with most of the file? I'll handle the other case in bug 826689.

Comment 5

[:Gijs (he/him)]

Attachment: Patch v2, fixed nit, included luna-blue

Oops about XP Luna blue, fixed nit.

Comment 6

[:Gijs (he/him)]

(In reply to Matthew N. [:MattN] from comment #4)
> Comment on attachment 741898 [details] [diff] [review]
> Patch
> 
> Review of attachment 741898 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It would be good to make sure this will align with what's needed for bug
> 850918 so we don't just end up reverting this. Would you mind taking that
> bug and getting review on it before landing this?

These don't look like they'll interfere. Can we land this one on UX and move on? :-)

Comment 7

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #6)
> (In reply to Matthew N. [:MattN] from comment #4)
> > Comment on attachment 741898 [details] [diff] [review]
> > Patch
> > 
> > Review of attachment 741898 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > It would be good to make sure this will align with what's needed for bug
> > 850918 so we don't just end up reverting this. Would you mind taking that
> > bug and getting review on it before landing this?
> 
> These don't look like they'll interfere. Can we land this one on UX and move
> on? :-)

Discussed with Matt, pushed: http://hg.mozilla.org/projects/ux/rev/1cbed63f3eea

Comment 8

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/1cbed63f3eea