Customize toolbar sheet moves when selecting the show dropdown menu

RESOLVED FIXED in Firefox 3.7a5

Status

()

Firefox
Toolbars and Customization
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Fredrik Henrysson, Assigned: mstange)

Tracking

({regression})

Trunk
Firefox 3.7a5
All
Mac OS X
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 years ago
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.
(Reporter)

Updated

8 years ago
Version: unspecified → Trunk
Confirming - slight different manifestation of Bug 549445 but perhaps the same root cause.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

8 years ago
Duplicate of this bug: 553551
(Assignee)

Updated

8 years ago
Keywords: regression, regressionwindow-wanted
(Assignee)

Comment 3

8 years ago
This is a regression from bug 439134.
Blocks: 439134
Keywords: regressionwindow-wanted
(Assignee)

Updated

8 years ago
Assignee: nobody → mstange
Status: NEW → ASSIGNED
(Assignee)

Comment 4

8 years ago
Created attachment 443181 [details] [diff] [review]
v1 - move the panel on load, not on popupshown

This also fixes bug 549445.
Attachment #443181 - Flags: review?(gavin.sharp)
(Assignee)

Updated

8 years ago
Blocks: 549445
(Assignee)

Comment 5

8 years ago
Created attachment 443966 [details] [diff] [review]
v1

updated diff on top of bug 528884 (browser_customize.js changed)
Attachment #443181 - Attachment is obsolete: true
Attachment #443966 - Flags: review?(gavin.sharp)
Attachment #443181 - Flags: review?(gavin.sharp)
Seems to me like this code belongs in the frame-specific branch of customizeToolbar.js's onLoad() instead.
(Assignee)

Comment 7

8 years ago
You mean these four lines in the load handler? (the first would go away)

>+      sheetFrame.removeEventListener("load", arguments.callee, true);
>+      panel.moveTo(window.screenX + (window.innerWidth - panel.boxObject.width) / 2,
>+                   panel.boxObject.screenY);
>+      panel.style.removeProperty("visibility");

I agree that the positioning would fit better there due to symmetry with repositionDialog(). But I'd prefer to have setting and unsetting the visibility near each other - one shouldn't need to look into a different file to find the counterpart of a paired operation. Also, making customizeToolbar.js expect that it's hidden is a strange API.
What do you think about using the "beforecustomization" event for unhiding the panel in browser.js?
(In reply to comment #7)
> I agree that the positioning would fit better there due to symmetry with
> repositionDialog(). But I'd prefer to have setting and unsetting the visibility
> near each other - one shouldn't need to look into a different file to find the
> counterpart of a paired operation. Also, making customizeToolbar.js expect that
> it's hidden is a strange API.
> What do you think about using the "beforecustomization" event for unhiding the
> panel in browser.js?

Sounds reasonable.
(Assignee)

Comment 9

8 years ago
Created attachment 444172 [details] [diff] [review]
v2
Attachment #443966 - Attachment is obsolete: true
Attachment #444172 - Flags: review?(gavin.sharp)
Attachment #443966 - Flags: review?(gavin.sharp)
Comment on attachment 444172 [details] [diff] [review]
v2

This doesn't seem to work... It positions the window in the top left of the parent (slightly offscreen).
Attachment #444172 - Flags: review?(gavin.sharp) → review-
Comment on attachment 444172 [details] [diff] [review]
v2

Actually, I'm dumb and just forgot to rebuild correctly. :)
Attachment #444172 - Flags: review- → review+
(Assignee)

Comment 12

8 years ago
http://hg.mozilla.org/mozilla-central/rev/8eb86ffb2325
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a5

Updated

8 years ago
Blocks: 566424

Updated

8 years ago
Blocks: 566425
You need to log in before you can comment on or make changes to this bug.