Last Comment Bug 525404 - [Mac Classic] Appearance Pref Pane does not alter icon/text settings
: [Mac Classic] Appearance Pref Pane does not alter icon/text settings
Status: RESOLVED FIXED
: fixed-seamonkey2.0.1, regression
Product: SeaMonkey
Classification: Client Software
Component: Preferences (show other bugs)
: unspecified
: PowerPC Mac OS X
: -- normal (vote)
: seamonkey2.1a1
Assigned To: Stefan [:stefanh]
:
:
Mentors:
: 527221 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-29 22:19 PDT by Mark Dye
Modified: 2009-11-13 13:46 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Use new binding for .toolbar-primary (2.38 KB, patch)
2009-10-31 16:15 PDT, Stefan [:stefanh]
mnyromyr: review+
neil: superreview+
mnyromyr: approval‑seamonkey2.0.1+
Details | Diff | Splinter Review

Description Mark Dye 2009-10-29 22:19:31 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9.1.4) Gecko/20091017 SeaMonkey/2.0
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9.1.4) Gecko/20091017 SeaMonkey/2.0

Appearance pref panel does not change icon/text setting.  Also there is no "Done" button is the lower right corner, as in ver. 1.18

Reproducible: Always

Steps to Reproduce:
1. Open preferences, open "Appearance"
2. Change "Icons and Text" to either text or icons only
3. Close preference with red dot, no "Close" button available
4. No change immediately, no change after restart
Actual Results:  
No change to text or icons

Expected Results:  
Wanted to have text only
Comment 1 lenochod 2009-10-29 22:54:26 PDT
Mozilla/5.0 (Windows; U; Windows NT 5.1; cs; rv:1.9.1.4) Gecko/20091017 SeaMonkey/2.0

Works fine on Windows XP SP3 in Default theme.

Please try in Safe mode ( http://kb.mozillazine.org/Talk:Safe_Mode#Safe_Mode_in_SeaMonkey_2 ), or with a new profile ( http://kb.mozillazine.org/Profile_Manager#Creating_a_new_profile ).

What do you use theme?

Do you get any error/warning/message in the Error Console ?
Comment 2 Stefan [:stefanh] 2009-10-31 12:12:46 PDT
I tested this with 2.0 and my trunk build (mac). It seems that the appearance pref does nothing. Selecting the various radio button does change the pref value of browser.chrome.toolbar_style, but it seems it has no effect.
Comment 3 Stefan [:stefanh] 2009-10-31 12:13:45 PDT
(In reply to comment #0)

> Also there is no
> "Done" button is the lower right corner, as in ver. 1.18

That's by design - the prefs applies instantly, like they do in any mac application.
Comment 4 Stefan [:stefanh] 2009-10-31 12:40:21 PDT
I tested this with a clean profile and even if I've never touch any of the customize settings, the pref changes has no effect.
Comment 5 Stefan [:stefanh] 2009-10-31 13:16:19 PDT
OK, so in bug 465089 a listener was added in the grippytoolbar-primary binding. 
However, there's a -moz-binding rule in toolkit's global.css, that replaces the grippy-toolbar binding with toolkit's toolbar.xml#toolbar-drag. This is so that you can drag the window from the toolbar. Now, this is not enough - in order to get the grippies back (of course, they're now hidden by default in mac classic) another rule was added in mac classic's toolbar.css:

toolbar:not([nowindowdrag="true"]):not([type="menubar"]) {
-moz-binding: url("chrome://communicator/content/bindings/toolbar.xml#grippytoolbar-drag");
}

Philip, as I understand it, we only want the pref listener in a .toolbar-primary, right?
Comment 6 Stefan [:stefanh] 2009-10-31 13:17:25 PDT
Oh, btw - I should probably mention that it of course works in Modern.
Comment 7 Stefan [:stefanh] 2009-10-31 15:15:02 PDT
One solution is to just make a grippytoolbar-primary-drag binding that extends the grippytoolbar-drag binding. However, having 2 identical constructors doesn't seem optional...
Comment 8 Stefan [:stefanh] 2009-10-31 16:15:18 PDT
Created attachment 409551 [details] [diff] [review]
Use new binding for .toolbar-primary

I don't really see any other way here. Unless Karsten is OK with removing the grippy bindings for mac classic of course ;-)
Comment 9 Stefan [:stefanh] 2009-10-31 16:54:19 PDT
This wasn't really caused by bug 465089, so I'm removing it from the dependency list.
Comment 10 Stefan [:stefanh] 2009-11-07 11:01:26 PST
*** Bug 527221 has been marked as a duplicate of this bug. ***
Comment 11 Stefan [:stefanh] 2009-11-07 11:03:42 PST
Putting this on the 2.0.2 blocking radar so it doesn't get lost.
Comment 12 Stefan [:stefanh] 2009-11-07 11:04:18 PST
(In reply to comment #11)
> Putting this on the 2.0.2 blocking radar so it doesn't get lost.

uh, I ment 2.0.1
Comment 13 Karsten Düsterloh 2009-11-09 12:59:08 PST
Comment on attachment 409551 [details] [diff] [review]
Use new binding for .toolbar-primary

>diff --git a/suite/common/bindings/toolbar.xml b/suite/common/bindings/toolbar.xml
>--- a/suite/common/bindings/toolbar.xml
>+++ b/suite/common/bindings/toolbar.xml
>@@ -275,16 +275,34 @@
>               return !(this.parentNode.customizing || window.fullScreen);
>             }
>           } catch (e) {}
>         ]]>
>       </constructor>
>     </implementation>
>   </binding>
> 
>+  <binding id="grippytoolbar-primary-drag"
>+           extends="chrome://communicator/content/bindings/toolbar.xml#grippytoolbar-primary">
>+    <implementation>
>+      <constructor>
>+        <![CDATA[
>+          try {
>+            Components.utils.import("resource://gre/modules/WindowDraggingUtils.jsm");
>+            let draggableThis = new WindowDraggingElement(this, window);
>+            draggableThis.mouseDownCheck = function(e) {
>+            // Don't move while customizing or while in full screen mode.
>+              return !(this.parentNode.customizing || window.fullScreen);
>+            }
>+          } catch (e) {}
>+        ]]>
>+      </constructor>
>+    </implementation>
>+  </binding>
>+
>   <binding id="grippytoolbar-menubar"
>            extends="chrome://communicator/content/bindings/toolbar.xml#grippytoolbar"
>            display="xul:menubar"/>
> 
>   <binding id="grippymenubar" extends="chrome://global/content/bindings/toolbar.xml#menubar">
>     <content>
>       <xul:hbox flex="1" class="toolbar-box">
>         <xul:toolbargrippy xbl:inherits="last-toolbar,hidden=grippyhidden"
>diff --git a/suite/themes/classic/mac/communicator/toolbar.css b/suite/themes/classic/mac/communicator/toolbar.css
>--- a/suite/themes/classic/mac/communicator/toolbar.css
>+++ b/suite/themes/classic/mac/communicator/toolbar.css
>@@ -42,16 +42,20 @@
>   ======================================================================= */
> 
> @namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
> 
> toolbar:not([nowindowdrag="true"]):not([type="menubar"]) {
>   -moz-binding: url("chrome://communicator/content/bindings/toolbar.xml#grippytoolbar-drag");
> }
> 
>+.toolbar-primary:not([nowindowdrag="true"]):not([type="menubar"]) {
>+  -moz-binding: url("chrome://communicator/content/bindings/toolbar.xml#grippytoolbar-primary-drag");
>+}
>+
> toolbargrippy {
>   display: none;
>   -moz-box-orient: vertical;
>   -moz-box-align: center;
>   width: 10px;
>   padding: 2px 1px;
>   list-style-image: url("chrome://communicator/skin/toolbar/tbgrip-arrow.gif");
> }
Comment 14 DickB 2009-11-09 14:13:54 PST
Icon/text can be changed for the navigator window and the mail/news window by right clicking on the toolbar.

This also holds for the modern theme! However: after a change the settings [icons icons+text text ] in the pref-pane no longer affect one of these windows but affect only the composer window and the addressbook window.
Comment 15 Philip Chee 2009-11-09 17:11:12 PST
> This also holds for the modern theme! However: after a change the settings
> [icons icons+text text ] in the pref-pane no longer affect one of these windows
> but affect only the composer window and the addressbook window.

This is working as designed. Right click on a customized toolbar and go "Settings for this toolbar" -> "Use default settings". Control will then be handed back to those preferences.
Comment 16 Stefan [:stefanh] 2009-11-10 13:16:58 PST
http://hg.mozilla.org/comm-central/rev/a27d6c9eef94
Comment 17 Stefan [:stefanh] 2009-11-10 13:21:12 PST
Comment on attachment 409551 [details] [diff] [review]
Use new binding for .toolbar-primary

Hardly any risk since we just attach a missing binding. Worth taking because the issue is quite visible.

Note You need to log in before you can comment on or make changes to this bug.