Closed Bug 346931 Opened 18 years ago Closed 17 years ago

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

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: moco, Assigned: dao)

References

Details

Attachments

(2 files, 2 obsolete files)

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>
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.
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.
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
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
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → dao
Attachment #232083 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #265568 - Flags: review?(mano)
Attachment #265568 - Attachment is obsolete: true
Attachment #265570 - Flags: review?(mano)
Attachment #265568 - Flags: review?(mano)
Attached patch v2 diff -wSplinter Review
Flags: blocking-firefox3?
Comment on attachment 265570 [details] [diff] [review]
patch v2 (checked in)

r=mano
Attachment #265570 - Flags: review?(mano) → review+
Whiteboard: [checkin needed]
"patch v2" checked in. Clearing checkin-needed status.
Whiteboard: [checkin needed]
Attachment #265570 - Attachment description: patch v2 → patch v2 (checked in)
Status: ASSIGNED → RESOLVED
Closed: 17 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.

Attachment

General

Created:
Updated:
Size: