Last Comment Bug 566424 - [SM] Customize toolbar sheet moves when selecting the show dropdown menu
: [SM] Customize toolbar sheet moves when selecting the show dropdown menu
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: seamonkey2.1a2
Assigned To: Ian Neal
:
Mentors:
Depends on: 549562 566425
Blocks: 439134 549445
  Show dependency treegraph
 
Reported: 2010-05-17 13:04 PDT by Ian Neal
Modified: 2010-06-28 21:26 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
port fx patch v0.1 (2.71 KB, patch)
2010-05-17 13:06 PDT, Ian Neal
no flags Details | Diff | Splinter Review
less direct port of fx patch v0.1a [Checkin: Comment 9] (2.70 KB, patch)
2010-05-17 13:17 PDT, Ian Neal
mnyromyr: review+
neil: superreview+
stefanh: feedback+
Details | Diff | Splinter Review

Description Ian Neal 2010-05-17 13:04:20 PDT
+++ This bug was initially created as a clone of Bug #549562 +++

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a3pre) Gecko/20100301 Minefield/3.7a3pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a3pre) Gecko/20100301 Minefield/3.7a3pre

See summary

Reproducible: Always

Steps to Reproduce:
1. Bring up the customize toolbar dialog/sheet
2. Decide to change the show option. Therefore select the show dropdown menu.

Actual Results:  
Sheet moves right.

Expected Results:  
There should not be any movement at all.
Comment 1 Ian Neal 2010-05-17 13:06:52 PDT
Created attachment 445786 [details] [diff] [review]
port fx patch v0.1

This patch:
* Direct port of fx patch.
Comment 2 Ian Neal 2010-05-17 13:17:40 PDT
Created attachment 445792 [details] [diff] [review]
less direct port of fx patch v0.1a [Checkin: Comment 9]

Changes since v0.1:
* Use toolbox rather than gNavToolbox so that it works on mailnews as well as browser.
Comment 3 Stefan [:stefanh] 2010-05-17 13:35:54 PDT
Comment on attachment 445792 [details] [diff] [review]
less direct port of fx patch v0.1a [Checkin: Comment 9]

Yes, this fixes it for me.
Comment 4 Tony Mechelynck [:tonymec] 2010-05-19 02:26:44 PDT
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100510 SeaMonkey/2.1a1pre - Build ID: 20100510003927

This is billed as a Mac bug but I see it on Linux too.
Comment 5 Karsten Düsterloh 2010-06-06 15:20:50 PDT
Comment on attachment 445792 [details] [diff] [review]
less direct port of fx patch v0.1a [Checkin: Comment 9]

I don't see this under Linux, only on Mac (where it is quite surprising *g*).

>diff --git a/suite/common/utilityOverlay.js b/suite/common/utilityOverlay.js
>   if (gCustomizeSheet) {
>     var sheetFrame = document.getElementById("customizeToolbarSheetIFrame");
>+    var panel = document.getElementById("customizeToolbarSheetPopup");
>     sheetFrame.hidden = false;
>     sheetFrame.toolbox = toolbox;
>+    sheetFrame.panel = panel;
> 
>     // The document might not have been loaded yet, if this is the first time.
>     // If it is already loaded, reload it so that the onload initialization
>     // code re-runs.
>     if (sheetFrame.getAttribute("src") == customizeURL)
>       sheetFrame.contentWindow.location.reload();
>     else
>       sheetFrame.setAttribute("src", customizeURL);
> 
>-    document.getElementById("customizeToolbarSheetPopup")
>-            .openPopup(toolbox, "after_start", 0, 0);
>-
>+    // Open the panel, but make it invisible until the iframe has loaded so
>+    // that the user doesn't see a white flash.
>+    panel.style.visibility = "hidden";
>+    toolbox.addEventListener("beforecustomization", function () {
>+      toolbox.removeEventListener("beforecustomization", arguments.callee, false);
>+      panel.style.removeProperty("visibility");
>+    }, false);
>+    panel.openPopup(toolbox, "after_start", 0, 0);
>     return sheetFrame.contentWindow;
>   }
>   else {
>     return window.openDialog(customizeURL,
>                              "",
>                              "chrome,all,dependent",
>                              toolbox);
>   }
>diff --git a/suite/common/utilityOverlay.xul b/suite/common/utilityOverlay.xul
>--- a/suite/common/utilityOverlay.xul
>+++ b/suite/common/utilityOverlay.xul
>@@ -111,19 +111,17 @@
>     <menuseparator id="toolbar-customize-sep"/>
>     <menuitem id = "customize_toolbars"
>               command="cmd_CustomizeToolbars"
>               label="&customizeToolbarContext.label;"
>               accesskey="&customizeToolbarContext.accesskey;"/>
> 
>   </popup>
> 
>-  <panel id="customizeToolbarSheetPopup"
>-         noautohide="true"
>-         onpopupshown="this.moveTo(this.boxObject.screenX + (window.innerWidth - this.boxObject.width) / 2, this.boxObject.screenY);">
>+  <panel id="customizeToolbarSheetPopup" noautohide="true">
>     <iframe id="customizeToolbarSheetIFrame"
>             style="&dialog.style;"
>             hidden="true"/>
>   </panel>
> 
>   <statusbarpanel id="offline-status" context="networkProperties"
>                   observes="Communicator:WorkMode"/>
>
Comment 6 Karsten Düsterloh 2010-06-06 15:21:33 PDT
Oops, sorry, just ignore the quoted part. :-[
Comment 7 neil@parkwaycc.co.uk 2010-06-15 02:33:22 PDT
(In reply to comment #5)
> (From update of attachment 445792 [details] [diff] [review])
> I don't see this under Linux, only on Mac (where it is quite surprising *g*).
I don't know about Linux but you can reproduce it on Windows by turning on the toolbar.customization.usesheet preference.
Comment 8 neil@parkwaycc.co.uk 2010-06-15 02:49:02 PDT
Comment on attachment 445792 [details] [diff] [review]
less direct port of fx patch v0.1a [Checkin: Comment 9]

>-  <panel id="customizeToolbarSheetPopup"
>-         noautohide="true"
>-         onpopupshown="this.moveTo(this.boxObject.screenX + (window.innerWidth - this.boxObject.width) / 2, this.boxObject.screenY);">
So, the problem was that the onpopupshown was bubbling up from the inner menulist. Oops.

>+    var panel = document.getElementById("customizeToolbarSheetPopup");
>     sheetFrame.hidden = false;
>     sheetFrame.toolbox = toolbox;
>+    sheetFrame.panel = panel;
The toolkit code solves this by moving the moving code to the load event (which we are triggering by setting the panel property).

>+    // Open the panel, but make it invisible until the iframe has loaded so
>+    // that the user doesn't see a white flash.
>+    panel.style.visibility = "hidden";
>+    toolbox.addEventListener("beforecustomization", function () {
>+      toolbox.removeEventListener("beforecustomization", arguments.callee, false);
>+      panel.style.removeProperty("visibility");
>+    }, false);
And this just makes opening the panel slicker :-) Although in about:rights we apparently use .style.<name> = "" instead of removeProperty.
Comment 9 Ian Neal 2010-06-15 13:35:42 PDT
Comment on attachment 445792 [details] [diff] [review]
less direct port of fx patch v0.1a [Checkin: Comment 9]

http://hg.mozilla.org/comm-central/rev/d2a6f0324abe
Comment 10 Tony Mechelynck [:tonymec] 2010-06-28 21:26:15 PDT
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a6pre) Gecko/20100628 Lightning/1.1a1pre SeaMonkey/2.1a3pre - Build ID: 20100628004420

"toolbar.customization.usesheet [user set] true" is part of my permanent about:config settings. Please make sure you have it on before you test this fix.

I've been noticing for a week or two already, that the toolbar customization sheet now drops from the middle of the bottommost toolbar (sometimes it appears first at left, possibly with blurred display, but everything gets straightened out within a second or so) -- not like what I saw in comment #4.

I VERIFY this fix for Linux 32-bit. Please check it as needed on other platforms.

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