Closed Bug 865698 Opened 9 years ago Closed 8 years ago

Avoid using generated content: "" as much as possible

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

(Whiteboard: [Australis:M3])

Attachments

(1 file, 1 obsolete file)

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.
(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.
Attached patch Patch (obsolete) — Splinter Review
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
Attachment #741898 - Flags: review?(mnoorenberghe+bmo)
(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.
Status: NEW → ASSIGNED
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.
Attachment #741898 - Flags: review?(mnoorenberghe+bmo) → review+
Oops about XP Luna blue, fixed nit.
Attachment #741898 - Attachment is obsolete: true
Attachment #743671 - Flags: review+
(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? :-)
(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
Whiteboard: [Australis:M3] → [Australis:M3][fixed-in-ux]
https://hg.mozilla.org/mozilla-central/rev/1cbed63f3eea
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M3][fixed-in-ux] → [Australis:M3]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.