Add more elements into tabbrowser tabs for easier stylability

RESOLVED FIXED in Firefox 4.0b7

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: mstange, Assigned: dao)

Tracking

Trunk
Firefox 4.0b7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

7 years ago
Created attachment 472611 [details] [diff] [review]
v1

This is needed for a better tab appearance, e.g. bug 547787 and bug 570279.

This patch has two parts: the actual addition of elements in tabbrowser.xml, and a mechanical change to all instances of browser.css and tabbrowser.css, which is just an automatic replacement of " > .tab-" with " > * > * > .tab-".
Attachment #472611 - Flags: review?(dao)
(Reporter)

Comment 1

7 years ago
Created attachment 472756 [details] [diff] [review]
v2

updated to trunk
Attachment #472611 - Attachment is obsolete: true
Attachment #472756 - Flags: review?(dao)
Attachment #472611 - Flags: review?(dao)
blocking2.0: --- → ?
(Reporter)

Comment 2

7 years ago
This is necessary for blocking bug 547787, and it affects add-on compatibility, so it needs to block beta6.
(Assignee)

Comment 3

7 years ago
Comment on attachment 472756 [details] [diff] [review]
v2

Let's clean this up a bit and make it more forward-compatible by adding tabbrowser specific classes and inherting some attributes where it makes sense (preferably with a "tab" prefix, I think, e.g. fadein as tabfadein -- although fadein may be a case where inheriting isn't worth it).
Attachment #472756 - Flags: review?(dao) → review-
(Reporter)

Comment 4

7 years ago
Can you write the patch that does that? You know this code much better than I do.
Blocks a blocker -> blocking+.
blocking2.0: ? → betaN+
(Assignee)

Updated

7 years ago
Assignee: mstange → dao
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Updated

7 years ago
Depends on: 593690
(Assignee)

Comment 6

7 years ago
Created attachment 475051 [details] [diff] [review]
wip

Only tested on Linux so far.

This doesn't add tab-background-*. This needs to be done in bug 547787.
Attachment #472756 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Blocks: 544818
(Assignee)

Comment 7

7 years ago
Created attachment 475089 [details] [diff] [review]
patch
Attachment #475051 - Attachment is obsolete: true
Attachment #475089 - Flags: review?(gavin.sharp)
(Assignee)

Comment 8

7 years ago
tab-icon-image and tab-close-button are already tabbrowser-specific, so I didn't have to invent new classes. For the label I chose tab-label instead of tabbrowser-tab-label or tabbrowser-tab-text as it would be the only class following a tabbrowser-tab-* pattern.
Comment on attachment 475089 [details] [diff] [review]
patch

>diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul

>         <toolbarbutton id="allTabs-tab-close-button"
>-                       class="tab-close-button"
>+                       class="tabs-closebutton"

I don't really understand what the differences between these two are supposed to be... tab-close-button is for the buttons on the tabs, and tabs-closebutton is for all the others? Why doesn't this one want to be like the button on the tab? Wish we'd used more distinctive names...

There are .tab-close-button and .tab-label rules in both browser.css and tabbrowser.css, and tab-icon-image rules are only in browser.css - shouldn't this be more consistent?

>diff --git a/browser/base/content/tabbrowser.css b/browser/base/content/tabbrowser.css

>-.tabbrowser-tab[pinned] > .tab-icon-image {
>+.tab-stack {
>+  vertical-align: middle; /* for pinned tabs */

Why change to vertical-aligning the entire stack rather than just the image?
Attachment #475089 - Flags: review?(gavin.sharp) → review+
We should probably get this in sooner rather than later (i.e. in b7).
blocking2.0: betaN+ → beta7+
(Assignee)

Comment 11

7 years ago
(In reply to comment #9)
> I don't really understand what the differences between these two are supposed
> to be... tab-close-button is for the buttons on the tabs, and tabs-closebutton
> is for all the others? Why doesn't this one want to be like the button on the
> tab? Wish we'd used more distinctive names...

tabs-closebutton was really supposed to be used only for the tab close button at the end of the tab strip, but it leaked into different places.

> There are .tab-close-button and .tab-label rules in both browser.css and
> tabbrowser.css, and tab-icon-image rules are only in browser.css - shouldn't
> this be more consistent?

Yes, I tried to slightly improve it by leaving only the rules related to the animation in browser.css...

> >diff --git a/browser/base/content/tabbrowser.css b/browser/base/content/tabbrowser.css
> 
> >-.tabbrowser-tab[pinned] > .tab-icon-image {
> >+.tab-stack {
> >+  vertical-align: middle; /* for pinned tabs */
> 
> Why change to vertical-aligning the entire stack rather than just the image?

Just the image doesn't work, as the image's parent isn't display:block, but the stack's parent is.
(Assignee)

Comment 12

7 years ago
http://hg.mozilla.org/mozilla-central/rev/875f1912a091
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b7
(Reporter)

Comment 13

7 years ago
Thanks Dão!

Updated

7 years ago
No longer blocks: 544818
(Assignee)

Updated

7 years ago
Blocks: 544818

Updated

7 years ago
Depends on: 601228
You need to log in before you can comment on or make changes to this bug.