Closed
Bug 112699
Opened 23 years ago
Closed 22 years ago
changing the "Hide tab bar with only one tab" pref is not immediate
Categories
(SeaMonkey :: Tabbed Browser, defect)
SeaMonkey
Tabbed Browser
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: bugzilla, Assigned: caillon)
References
Details
Attachments
(1 file, 6 obsolete files)
8.68 KB,
patch
|
bzbarsky
:
review+
jag+mozilla
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
seen using 2001.11.28.0x-comm bits on linux rh7.2, mac 10.1.1 and winnt. 0. make sure you have only one tab displayed in the browser window --or if you have the tab bar closed [default state], make sure you don't have multiple tabs displayed. 1. open the Preferences dialog and go to the Tabbed Browser panel. 2. toggle the state of the "Hide the tab bar when only one tab is open" checkbox [turn it off if it's on, or vice versa]. 3. click OK to save and dismiss the Prefs dlg. expected: if you had the tab bar open previously, the tab bar should go away after dismissing the Prefs dlg. conversely, if the tab bar was closed, it should appear after dismissing the Prefs dlg. actual results: after dismissing the Prefs dlg, there's no change wrt to the tab bar. the changes only appear after the following workarounds: * if the tab bar was previously open, i need to open another tab after dismissing Prefs, close it, then the tab bar closes. * if the tab bar was previously closed, i need to open another tab after dismissing Prefs, close it, then the tab bar remains open.
Reporter | ||
Comment 1•23 years ago
|
||
*** Bug 106636 has been marked as a duplicate of this bug. ***
Comment 3•23 years ago
|
||
-> future, helpwanted It shouldn't be too hard to fix this, implement a pref listener similar to the button pref listener, and make it open/close the bar as appropriate. cc'ing some people who might want to take a stab at this, or know people who might want to.
Keywords: helpwanted
Target Milestone: --- → Future
Assignee | ||
Comment 5•22 years ago
|
||
Assignee | ||
Updated•22 years ago
|
Comment 6•22 years ago
|
||
Um... What if one uses <browser>, not <tabbrowser>? That code looks like it assumes that gBrowser.mTabContainer exists and the like... Any way to move this code over into tabbrowser.xml?
Assignee | ||
Comment 7•22 years ago
|
||
/me mutters something about bug 50349...
Attachment #71061 -
Attachment is obsolete: true
Assignee | ||
Comment 8•22 years ago
|
||
Comment on attachment 71252 [details] [diff] [review] Patch v2.0 actually this patch sucks too. It creates a new object on each access. I'll get a better one out.
Attachment #71252 -
Attachment is obsolete: true
Assignee | ||
Comment 9•22 years ago
|
||
Comment 10•22 years ago
|
||
Comment on attachment 71277 [details] [diff] [review] Proposed fix v3.0 >+ this.observe = handler; This line is not needed -- you set this.observe below.... r=bzbarsky with that change
Attachment #71277 -
Flags: review+
Assignee | ||
Comment 11•22 years ago
|
||
Back to square 1 with a few tweaks. Implements generic add/remove observer functions as per jag. jag, bz this should be ready to go! :)
Attachment #71277 -
Attachment is obsolete: true
Comment 12•22 years ago
|
||
Comment on attachment 71402 [details] [diff] [review] Patch v1.1 r=bzbarsky, assuming you've tested this and it works and all that
Attachment #71402 -
Flags: review+
Comment 13•22 years ago
|
||
Comment on attachment 71402 [details] [diff] [review] Patch v1.1 >Index: mozilla/xpfe/browser/resources/content/navigator.js >=================================================================== >RCS file: /cvsroot/mozilla/xpfe/browser/resources/content/navigator.js,v >retrieving revision 1.433 >diff -u -r1.433 navigator.js >--- mozilla/xpfe/browser/resources/content/navigator.js 2002/02/20 04:06:57 1.433 >+++ mozilla/xpfe/browser/resources/content/navigator.js 2002/02/26 01:14:01 >@@ -207,18 +207,29 @@ // ... >+var nsButtonPrefListener = new Function(); > nsButtonPrefListener.prototype = I'm wondering if we should be using: var nsButtonPrefListener = { // ... } // ... window.gButtonPrefListener = nsButtonPrefListener; Since you'll only have one copy of this object per window. > { > domain: "browser.toolbars.showbutton", >@@ -247,6 +258,22 @@ > } > } > >+var nsTabStripPrefListener = new Function(); >+nsTabStripPrefListener.prototype = >+{ >+ domain: "browser.tabs.autoHide", >+ observe: function(subject, topic, prefName) >+ { >+ // verify that we're changing the tab browser strip auto hide pref >+ if (topic != "nsPref:changed") return; >+ if (prefName != this.domain) return; Please put the return on the next line. Do we need to check if |prefName != this.domain|, considering that that particular listener is only registered for |this.domain|?
Assignee | ||
Comment 14•22 years ago
|
||
Move the const var foo = { /* ... */ } declarations to the beginning of the file with other var declarations. Fix up other issues.
Attachment #71402 -
Attachment is obsolete: true
Comment 15•22 years ago
|
||
Comment on attachment 72248 [details] [diff] [review] Patch v1.2 r=bzbarsky
Attachment #72248 -
Flags: review+
Comment 16•22 years ago
|
||
Comment on attachment 72248 [details] [diff] [review] Patch v1.2 >+ var show = pref.getBoolPref(prefName); >+ if (show) >+ button.setAttribute("hidden","false"); >+ else >+ button.setAttribute("hidden", "true"); button.hidden = !show; >+ >+ // If all the buttons before the separator are hidden, also hide the >+ // separator >+ if(allLeftButtonsAreHidden()) >+ document.getElementById("home-bm-separator").setAttribute("hidden", "true"); >+ else >+ document.getElementById("home-bm-separator").removeAttribute("hidden"); Or rather: var bookmarkSeparator = document.getElementById("home-bm-separator"); bookmarkSeparator.hidden = allLeftButtonsAreHidden(); I know the above is not your doing, but since you're moving this code, please fix these nits :-) sr=jag
Attachment #72248 -
Flags: superreview+
Assignee | ||
Comment 17•22 years ago
|
||
Fix jag's nits and address a case where starting Moz with the auto hide pref set to true, then setting it to false will not initialize the tab listeners and leave you with broken tabbed browsing.
Assignee | ||
Updated•22 years ago
|
Attachment #72248 -
Attachment is obsolete: true
Assignee | ||
Comment 18•22 years ago
|
||
Reindents code as requested by jag ;)
Attachment #72552 -
Attachment is obsolete: true
Comment 19•22 years ago
|
||
Comment on attachment 72554 [details] [diff] [review] Patch v1.4 sr=jag
Attachment #72554 -
Flags: superreview+
Comment 20•22 years ago
|
||
Comment on attachment 72554 [details] [diff] [review] Patch v1.4 r=bzbarsky
Attachment #72554 -
Flags: review+
Comment 21•22 years ago
|
||
Comment on attachment 72554 [details] [diff] [review] Patch v1.4 a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #72554 -
Flags: approval+
Comment 22•22 years ago
|
||
checked in on the trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 23•22 years ago
|
||
vrfy'd fixed using 2002.03.13 comm bits on linux rh7.2, win2k and mac 10.1.3.
Status: RESOLVED → VERIFIED
Comment 24•22 years ago
|
||
*** Bug 168421 has been marked as a duplicate of this bug. ***
Updated•16 years ago
|
Product: Core → SeaMonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•