User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1 Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:5.0a2) Gecko/20110520 Firefox/5.0a2 ID:20110520042001 Bug 574654, which landed in Firefox 4.0, removed browser.tabs.tabMinWidth and browser.tabs.tabMaxWidth in favor of CSS min/max width style rules. As part of this bug, it was advised to use the custom tab width addon (https://addons.mozilla.org/en-US/firefox/addon/custom-tab-width/) by Dao Gottwald. Other addons like Classic Compact Options (https://addons.mozilla.org/en-US/firefox/addon/classic-compact-options/) followed Dao's suggested model for restoring the ability to customize the min & max widths of tabs (a feature CCO has had since FF3.0). Now Bug 465086 has landed in FF5a2 to stop tabs from resizing when tabs are closed until the cursor leaves the tab bar. The problem is, the "fix" in bug 465086 stops working if Dao's solution is applied to restore functionality lost because of bug 574654. Given that bug 574654 and the suggested solution to restore functionality lost in bug 574654 landed first in FF4. The "fix" in bug 465086 needs to be reworked to accommodate Dao's min/max width CSS solution. Reproducible: Always Steps to Reproduce: 1. Install the Custom Tab Width add-on https://addons.mozilla.org/en-US/firefox/addon/custom-tab-width/ 2. customize min and max tab widths 3. Open a bunch of tabs. 4. Start closing tabs Actual Results: Tab widths jump around Expected Results: Tab widths stay the same.

As a theme and extension developer, I find bugs like this extremely infuriating due to the amount of time I end up wasting trying to figure out what happened. In this case I first had to contend with bug 57465 which broke functionality in FF4 users of my extension Classic Compact Options have come to expect. Figuring out what was going on, finding then reading up on bug 57465 and then working out a fix based on Dao's extension took hours of time. Now in FF5 a bug 465086 breaks when the recommended fix for bug 57465 is used. I spent the better part of five hours just trying to figure out what was going on. The big problem with the conflict created by these two bugs is that I'll have one group of users who will want their min/max tab width settings like they've had for years. I'll have another group of users who want the new tab behavior where they don't resize right away. AND I'll have another group of users who will expect both at the same time. In short, a pair of bug "fixes" cause a conflict that 1) cost me many hours of development time just to figure out what is going on and 2) create a conflict that I may not be able to easily resolve. These types of things happen all the time with changes made in Firefox and it makes developing themes and extensions extremely frustrating.

Sorry, the above comment had a bad copy and paste of bug numbers. The two bugs being referenced are bug 574654 and bug 465086

This is not a bug in Firefox, since we don't guarantee a stable method (API) for changing the min- and max-widths of tabs.

CC'ing Dão, since he suggested the original workaround. Feel free to un-CC yourself if you're not interested, Dão.

(In reply to comment #3) > This is not a bug in Firefox, since we don't guarantee a stable method (API) > for changing the min- and max-widths of tabs. This is a valid bug and a valid complaint. Claiming it isn't is pretty cheeky way of washing hands of an issue. Claiming there is no guarantee of API stability gives an open license to break (intentionally or accidentally) any API due to shortcuts or incomplete testing and then blame everyone else. In the end, this is a disservice to end users, who then blame add-on developers for issues that are caused by short sighted "fixes" inside of Firefox. I'm tracking at least a half dozen bugs that break things in themes or extensions due to shortcuts taken by a Firefox developer who wasn't thinking past the immediate thing they were working on. We all are responsible for ensuring a good user experience and part of this includes a certain level of API stability. This certainly means Firefox tab behavior not breaking when a W3C standard style rule like min-width and max-width are used. The solution to bug 465086 was half baked and the desired behavior breaks when a perfectly legitimate style rule is applied. Rather than saying "we don't make guarantees," Efforts should be made to fix Firefox to properly handle min/max width CSS rules when applied to tabs.

(In reply to comment #0) > Steps to Reproduce: > 1. Install the Custom Tab Width add-on > https://addons.mozilla.org/en-US/firefox/addon/custom-tab-width/ > 2. customize min and max tab widths > 3. Open a bunch of tabs. > 4. Start closing tabs > > Actual Results: > Tab widths jump around > > Expected Results: > Tab widths stay the same. I can't reproduce this.

(In reply to comment #6) > (In reply to comment #0) > > Steps to Reproduce: > > 1. Install the Custom Tab Width add-on > > https://addons.mozilla.org/en-US/firefox/addon/custom-tab-width/ > > 2. customize min and max tab widths > > 3. Open a bunch of tabs. > > 4. Start closing tabs > > > > Actual Results: > > Tab widths jump around > > > > Expected Results: > > Tab widths stay the same. > > I can't reproduce this. I had the same problem when it was first brought to my attention. To reproduce it have enough tabs open that they start to reduce in size, but not so many that they are scrolling. Once they hit the min-width and start to scroll their width won't change when the tab is closed. Also make sure to use customized min/max widths (e.g. 100px and 275px). This is only a problem on FF5 & FF6 as this is where bug 465086 landed. I'm trying to figure out what exactly triggers the problem as Tab Mix Plus, which also sets min/max widths doesn't appear to have this problem. TMP doesn't use a style sheet to set min/max widths the way you do. Instead it sets these attributes directly using JavaScript. I'm wondering if the fix for 465086 gets broken by using "!important" on the style rules (I'm trying to test this).

(In reply to comment #7) > I had the same problem when it was first brought to my attention. To > reproduce it have enough tabs open that they start to reduce in size, but > not so many that they are scrolling. Once they hit the min-width and start > to scroll their width won't change when the tab is closed. Also make sure to > use customized min/max widths (e.g. 100px and 275px). It's still not clear to me what you're seeing. "their width won't change when the tab is closed" sounds like the goal of bug 465086.

Using your add-on or my add-on, Classic Compact Options, min/max tab width option, the width of tabs change as tabs are closed. With the patch that landed for bug 465086, the widths are not supposed to change until AFTER the cursor leaves the tab bar. SUMMARY: FF5 desired behavior: no tab width change as tabs are closed CTW1.0.1 and CCO behavior: tab widths change as tabs are closed. CAUSE: Setting "!important" to max-width (In CTW at line 4 chrome://tab-width/content/browser.css) overrides max-width setting used by patch for bug 465086. SOLUTION: There needs to be a two part solution. Part one is in Firefox itself to prevent add-on developers from accidentally breaking the new tab behavior. Part two are changes that need to be applied to add-ons to not break the new behavior. 1) In Firefox, the function that stops tab width changes needs to set "!important" to its max-width instruction while it is operational and/or set a max width attribute instead of using CSS. The goal is to always override any max-width set by add-ons. 2) Add-ons shouldn't use !important to set max-width of tabs. This is a pretty esoteric issue and I'm sure add-on developers will trip up against it from time to time. #1 above will help prevent innocent conflicts and avoid lots of frustration by add-on developers. It took me hours and hours of work to figure out what was causing the conflict with bug 465086.

> 1) In Firefox, the function that stops tab width changes needs to set > "!important" to its max-width instruction while it is operational and/or set > a max width attribute instead of using CSS. The goal is to always override > any max-width set by add-ons. > > 2) Add-ons shouldn't use !important to set max-width of tabs. Not using !important makes it depend on the stylesheet order -- I'm not sure if that's good. Anyway, 1) sounds right and should resolve this, so we don't need 2). Would you like to create the patch?

I'd love to create a patch, I'm just not sure I know how to. I was looking at what was changed in bug 465086 earlier and I might be able to hone in on what to fix. The challenge is I've never actually provided any fixes for Firefox itself. This evening I'll see what I can do.

See https://bugzilla.mozilla.org/show_bug.cgi?id=465086#c232.

I've never posted a patch before so please forgive any styling inconsistencies. The actual fix replaces "tab.style.maxWidth = tabWidth;" at line 2867 of "/browser/base/content/tabbrowser.xml" with "tab.style.cssText = 'max-width:'+tabWidth+' !important';". I tested this by hacking the omni.jar in my install of Aurora (Mozilla/5.0 (Windows NT 5.1; rv:5.0a2) Gecko/20110520 Firefox/5.0a2 ID:20110520042001) and tested against Custom Tab Width 1.0.1. It worked very smoothly. Here goes my attempt to use proper notation I've seen here in other bugs: --- a/browser/base/content/tabbrowser.xml +++ b/browser/base/content/tabbrowser.xml @@ line 2855 // Force tabs to stay the same width, unless we're closing the last tab, // which case we need to let them expand just enough so that the overall // tabbar width is the same. if (isEndTab) { let numNormalTabs = tabs.length - numPinned; tabWidth = tabWidth * (numNormalTabs + 1) / numNormalTabs; if (tabWidth > this._tabDefaultMaxWidth) tabWidth = this._tabDefaultMaxWidth; } tabWidth += "px"; for (let i = numPinned; i < tabs.length; i++) { let tab = tabs[i]; - tab.style.maxWidth = tabWidth; + tab.style.cssText = "max-width:"+tabWidth+" !important"; if (!isEndTab) { // keep tabs the same width tab.style.MozTransition = "none"; tab.clientTop; // flush styles to skip animation; see bug 649247 tab.style.MozTransition = ""; } }

A slight modification to my solution above. In order to prevent overriding someone else's style rules it might be wised to use the following line of code instead of what I originally proposed: + tab.style.cssText +=';max-width:'+tabWidth+' !important;'; The "+=" appends the style rule instead of overwriting existing ones. Leading and trailing semicolons make sure there is proper separation in style rules.

It took quite a bit of work to figure out how to install and use Mercurial so that I could generate this patch, but here it is. I tested this patch to make sure it did not erase other CSS rules that might have already been placed in the style attribute by some unknown add-on, etc. I also tested against Custom Tab Width 1.0.1 and Tab Mix Plus as they approach setting the max tab width option in two different ways. In all cases this patch caused tabs to behave as expected by Bug 465086 and without breaking custom max-width settings by add-ons.

Comment on attachment 534277 [details] [diff] [review] Make max-width style important Please use style.setProperty("max-width", ..., "important") instead of style.cssText.

Switched to setProperty() as advised. Tested against the add-ons Tab Mix Plus, Custom Tab Width & Classic Compact Options by hacking omni.jar of my Aurora install. Worked like a charm.

Comment on attachment 534300 [details] [diff] [review] Fix tab maxwidth style to !important using setProperty() Thanks!

Comment on attachment 534300 [details] [diff] [review] Fix tab maxwidth style to !important using setProperty() Review of attachment 534300 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/tabbrowser.xml @@ +2891,4 @@ > tabWidth += "px"; > for (let i = numPinned; i < tabs.length; i++) { > let tab = tabs[i]; > + tab.style.setProperty("max-width",tabWidth,"important"); Space please.

Ah, one step later :)

@Dão, Thanks for your help and support. This was my first patch submission. Also thanks for creating your Custom Tab Width add-on. It dug me out of a deep hole with my extension Classic Compact Options when bug 574654 landed.

Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110630 Firefox/7.0a1 Verified issue using steps from Comment 0. Problem no longer reproducible -> setting status to Verified Fixed.