Closed Bug 566424 Opened 14 years ago Closed 14 years ago

[SM] Customize toolbar sheet moves when selecting the show dropdown menu

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1a2

People

(Reporter: iannbugzilla, Assigned: iannbugzilla)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

+++ 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.
Attached patch port fx patch v0.1 (obsolete) — Splinter Review
This patch:
* Direct port of fx patch.
Attachment #445786 - Flags: review?(mnyromyr)
Attachment #445786 - Flags: feedback?(stefanh)
Depends on: 566425
Attachment #445786 - Flags: review?(mnyromyr)
Attachment #445786 - Flags: feedback?(stefanh)
Changes since v0.1:
* Use toolbox rather than gNavToolbox so that it works on mailnews as well as browser.
Attachment #445786 - Attachment is obsolete: true
Attachment #445792 - Flags: review?(mnyromyr)
Attachment #445792 - Flags: feedback?(stefanh)
Comment on attachment 445792 [details] [diff] [review]
less direct port of fx patch v0.1a [Checkin: Comment 9]

Yes, this fixes it for me.
Attachment #445792 - Flags: feedback?(stefanh) → feedback+
Summary: Customize toolbar sheet moves when selecting the show dropdown menu → [SM] Customize toolbar sheet moves when selecting the show dropdown menu
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.
OS: Mac OS X → All
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"/>
>
Attachment #445792 - Flags: review?(mnyromyr) → review+
Oops, sorry, just ignore the quoted part. :-[
Attachment #445792 - Flags: superreview?(neil)
(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 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.
Attachment #445792 - Flags: superreview?(neil) → superreview+
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
Attachment #445792 - Attachment description: less direct port of fx patch v0.1a → less direct port of fx patch v0.1a [Checkin: Comment 9]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1a2
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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: