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)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: bugzilla, Assigned: caillon)

References

Details

Attachments

(1 file, 6 obsolete files)

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.
*** Bug 106636 has been marked as a duplicate of this bug. ***
Reassigning to new component owner.
Assignee: hyatt → jaggernaut
-> 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
Taking.
Assignee: jaggernaut → caillon
Keywords: helpwanted
Status: NEW → ASSIGNED
Keywords: patch, review
Target Milestone: Future → ---
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?
Attached patch Patch v2.0 (obsolete) — Splinter Review
/me mutters something about bug 50349...
Attachment #71061 - Attachment is obsolete: true
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
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+
Attached patch Patch v1.1 (obsolete) — Splinter Review
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 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 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|?
Attached patch Patch v1.2 (obsolete) — Splinter Review
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 on attachment 72248 [details] [diff] [review]
Patch v1.2

r=bzbarsky
Attachment #72248 - Flags: review+
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+
Attached patch Patch v1.3 (obsolete) — Splinter Review
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.
Attachment #72248 - Attachment is obsolete: true
Attached patch Patch v1.4Splinter Review
Reindents code as requested by jag ;)
Attachment #72552 - Attachment is obsolete: true
Comment on attachment 72554 [details] [diff] [review]
Patch v1.4

sr=jag
Attachment #72554 - Flags: superreview+
Comment on attachment 72554 [details] [diff] [review]
Patch v1.4

r=bzbarsky
Attachment #72554 - Flags: review+
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+
checked in on the trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
vrfy'd fixed using 2002.03.13 comm bits on linux rh7.2, win2k and mac 10.1.3.
Status: RESOLVED → VERIFIED
*** Bug 168421 has been marked as a duplicate of this bug. ***
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: