code cleanup: remove <xul:hbox style="position: relative;"> from tabbrowser.xml and *stripe/global/globalBindings.xml

RESOLVED FIXED

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: dao)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

code cleanup:  remove <xul:hbox style="position: relative;"> from tabbrowser.xml and pinstripe/global/globalBindings.xml

it looks like roc has a fix, and once it lands (on trunk and/or branch) I can clean up this hack

2472         <!-- XXXsspitzer hack
2473              this extra hbox with position: relative
2474              is needed to work around two bugs.
2475              see bugs #346307 and #346035 
2476         -->
2477         <xul:hbox style="position: relative;">
2478           <xul:hbox flex="1" class="tabs-alltabs-box" anonid="alltabs-box"/>
2479         </xul:hbox>
2480         <!-- XXXsspitzer hack
2481              this extra hbox with position: relative
2482              is needed to work around two bugs.
2483              see bugs #346307 and #346035 
2484         -->
2485         <xul:hbox style="position: relative;">
2486           <xul:toolbarbutton class="tabs-alltabs-button" type="menu">
2487             <xul:menupopup class="tabs-alltabs-popup"
2488                            anonid="alltabs-popup" 
2489                            position="after_end"/>
2490         </xul:toolbarbutton>
Created attachment 232083 [details] [diff] [review]
patch, remove my hack now that roc has fixed bug #346035

Comment 2

12 years ago
Does this bug refer to the spacer box below scrollbutton-down?

From what I understand, once this bug is fixed, the box will no longer be present, such that if .scrollbutton-down is set to "display:none", then this space will be freely available to tabs, correct?

Also, does this spacer have a fixed width, or can its width be manually altered?

The reason I ask this is because if I could place a spacer there where this hbox resides that gave me the ability to manually select its width in pixels, then when I have removed scrollbutton-up and scrollbutton-down, I could correct for the offset problem that occurs when you have a set of fixed width 250 pixel tabs scrolled all the way to the left and all the way to the right, allowing for the close buttons on each tab to always be in the same constant place.

If the current hbox is removed, it would be nice to have the ability to add back some sort of  spacer there that can have its width controlled manually to correct for tab width disparity whenever the scrollbuttons are removed.

Comment 3

12 years ago
On further consideration of my previous message, a spacer is probably not best way of going about fixing this disparity.  Instead, choosing the appropriate width for each tab that is cleanly divisible into the full width of the tab bar, setting maxwidth and minwidth to correspond to this number, and removing .tabs-left and .scrollbutton-down-stack, is probably the best solution to the problem, and off-hand it seems to all be doable by editing userChrome.css.  I will keep my eyes peeled to the result of fixing this bug, however.
Seth, you need to ask review on the patch, not?
Bug 360474 mentions that there is also some code in the winstripe globalBindings.xml that can be removed.
(Assignee)

Updated

11 years ago
Flags: blocking-firefox3?
sorry for the bug spam, re-assigning bugs back to default owner if I'm not working actively on them.
Assignee: sspitzer → nobody
(Assignee)

Updated

11 years ago
Summary: code cleanup: remove <xul:hbox style="position: relative;"> from tabbrowser.xml and pinstripe/global/globalBindings.xml → code cleanup: remove <xul:hbox style="position: relative;"> from tabbrowser.xml and *stripe/global/globalBindings.xml
(Assignee)

Updated

11 years ago
Duplicate of this bug: 360474
(Assignee)

Comment 8

11 years ago
Created attachment 265568 [details] [diff] [review]
patch
Assignee: nobody → dao
Attachment #232083 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #265568 - Flags: review?(mano)
(Assignee)

Comment 9

11 years ago
Created attachment 265570 [details] [diff] [review]
patch v2 (checked in)
Attachment #265568 - Attachment is obsolete: true
Attachment #265570 - Flags: review?(mano)
Attachment #265568 - Flags: review?(mano)
(Assignee)

Comment 10

11 years ago
Created attachment 265572 [details] [diff] [review]
v2 diff -w
(Assignee)

Updated

11 years ago
Flags: blocking-firefox3?
Comment on attachment 265570 [details] [diff] [review]
patch v2 (checked in)

r=mano
Attachment #265570 - Flags: review?(mano) → review+
(Assignee)

Updated

11 years ago
Whiteboard: [checkin needed]

Comment 12

11 years ago
"patch v2" checked in. Clearing checkin-needed status.
Whiteboard: [checkin needed]

Updated

11 years ago
Attachment #265570 - Attachment description: patch v2 → patch v2 (checked in)
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Thanks dao for fixing this.  The less "XXXsspitzer hack" in the tree, the better!
You need to log in before you can comment on or make changes to this bug.