Closed Bug 212540 Opened 18 years ago Closed 15 years ago

Toolbar Customization dialog will not stay where I place it

Categories

(Toolkit :: Toolbars and Toolbar Customization, defect)

defect
Not set
major

Tracking

()

VERIFIED FIXED

People

(Reporter: bzbarsky, Assigned: mossop)

References

Details

(Keywords: fixed1.8.1)

Attachments

(2 files, 2 obsolete files)

BUILD: Mozilla Firebird 0.6 and Mozila Firebird July 10 Linux nightly.

STEPS TO REPRODUCE:
1)  Start browser with a clean install and clean profile
2)  Open "View" menu
3)  Select Toolbars > Customize
4)  Move the resulting dialog 200px down and to the right
5)  Drag the URL bar from the toolbar to the dialog

EXPECTED RESULTS: URL bar disappears from toolbar and appears in dialog

ACTUAL RESULTS: expected results, plus the dialog repositions itself back where
I moved it away from

The problem here is that while it's ok to center the dialog when it first comes
up, it is _not_ ok to override the user's window placement decisions.  That
leads to severe user frustration and anger (notice the options we have in our
browser to prevent page authors from doing that), not to mention simple
inconvenience.

ccing djk, since he seems to be working on this sort of thing in bug 178079 (and
the behavior there should probably be modified to only reposition the dialog if
the user has not already repositioned it).
When you choolse view->toolbars->customize, the dialog appears to 'drop down'
from the toolbar, giving the illusion of attachment to the toolbar. That's what
bug 178079 addresses: keeping the illusion of 'attachment' to the bottom of the
toolbars when the main window is moved/resized.

I don't think the plan is to allow one to independantly resize or move the
customize 'sheet'; it was changed from a standard dialog to the current form by
Hewitt (as I recall).

With 20030611 on W98 (and an older MFB on WinXP), the dialog has no decorations.
I can't resize it, and can only move it independently using keyboard shortcuts.
In fact, if I understand the desired behavior of the dialog, the fact that I can
move it is a bug. :-}

If the customize... dialog is turned back into a 'proper' dialog box, then I
imagine several issues with it will go away. However, if the design decision is
to make it act like a 'sheet', then the move issue and bug 178079 should be
addressed.

Regards,
David

ps- Boris, the cropping of the dialog that you mentioned on mozillazine is
probably bug 171454
> When you choolse [sic] view->toolbars->customize, the dialog appears to 'drop
> down' from the toolbar

That's not what it looks like here.

> I don't think the plan is to allow one to independantly resize or move the
> customize 'sheet'

Somewhat obnoxious, given that it comes up in an inconvenient location.  In any
case, it's quite movable and resizable at the moment.

> With 20030611 on W98 (and an older MFB on WinXP), the dialog has no
> decorations.

It has a titlebar and resize handles with my window manager; I can post a
screenshot if you would like.  I guess that's a bug in the current design, then....

Taking QA Contact
QA Contact: asa → bugzilla
This bug also shows up in the Mac version of Firefox 1.0PR (v 0.10.1, Oct 01 2004 12:52pm).  The 
Toolbar Customization dialog box recenters itself on the main window after you change the setting of 
the "Show:" "Icons and Text" pop-up menu on the dialog.

Scott
-----
Assignee: hyatt → bugs
Assignee: bugs → nobody
QA Contact: bugzilla → toolbars
*** Bug 317010 has been marked as a duplicate of this bug. ***
Appears on windows as well.
OS: Linux → All
Hardware: PC → All
Component: Toolbars → Toolbars and Toolbar Customization
Product: Firefox → Toolkit
QA Contact: toolbars → toolbars
Version: unspecified → Trunk
Assignee: nobody → mossop
Blocks: 264489, 270044
I intend to do something about this.
Status: NEW → ASSIGNED
Attached patch Stop repositioning the dialog (obsolete) — Splinter Review
This patch makes sure that the dialog is only positioned when the window is opened and that position merely moves the window to just below the toolbar. The dialog size is fixed and localised as per bug 270044. Also the dialog size is persisted.
Attachment #215796 - Flags: first-review?
Attachment #215796 - Flags: first-review? → first-review?(mconnor)
(In reply to comment #8)
> This patch makes sure that the dialog is only positioned when the window is
> opened and that position merely moves the window to just below the toolbar. The
> dialog size is fixed and localised as per bug 270044. Also the dialog size is
> persisted.
> 

That should be the initial dialog size is set by the locale and subsequently the dialog displays at the size it was previously.

Mike, can you take a quick look and say if this behaviour makes sense?
(In reply to comment #9)
> (In reply to comment #8)
> > This patch makes sure that the dialog is only positioned when the window is
> > opened and that position merely moves the window to just below the toolbar. 
[...]
> Mike, can you take a quick look and say if this behaviour makes sense?

Took a longer look than perhaps you wanted, but yes, this behaviour makes sense. :) In the long term we might want to make this a sheet on OSX, but you pointed out that's bug 206649.
Comment on attachment 215796 [details] [diff] [review]
Stop repositioning the dialog


>   // Position the dialog touching the bottom of the toolbox and centered with 
>-  // it. We must resize the window smaller first so that it is positioned 
>-  // properly. 
>-  var screenX = gToolbox.boxObject.screenX + ((gToolbox.boxObject.width - kWindowWidth) / 2);
>+  // it.
>+  var screenX = gToolbox.boxObject.screenX + ((gToolbox.boxObject.width - parseInt(document.documentElement.style.width)) / 2);

zany line length here.  Fix that, oh, somehow. :)

sorry for the delay!
Attachment #215796 - Flags: first-review?(mconnor)
Attachment #215796 - Flags: first-review+
Attachment #215796 - Flags: approval-branch-1.8.1+
Attached patch final patchSplinter Review
Identical patch, just fixed the line length.

Carrying over review and approval.
Attachment #215796 - Attachment is obsolete: true
Attachment #223369 - Flags: first-review+
Attachment #223369 - Flags: approval-branch-1.8.1+
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Now on trunk and branch.
Keywords: fixed1.8.1
Using the latest branch build that includes this patch I don't see that this bug is fixed as the position of the "Toolbar Customization Dialog" is not remembered!

If I center the dialog (move to the center of my screen), click done and then View-> Toolbars-> Customize, the dialog is back to the default location and is no longer centered.

BUILD: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060527 BonEcho/2.0a3 ID:2006052715

~B
(In reply to comment #14)
> If I center the dialog (move to the center of my screen), click done and then
> View-> Toolbars-> Customize, the dialog is back to the default location and is
> no longer centered.

That is correct. The dialog is always positioned just beneath the toolbars to be customised when it is displayed. This bug was about stopping it being repositioned there every time you added/removed an icon from a toolbar.
If you extend the left edge of the dialog to the left edge of the screen, then close and reopen it, it is centered according to the original width, not the new width. I'm not really sure how to fix this, but I imagine a change to:

+  var screenX = gToolbox.boxObject.screenX 
+                + ((gToolbox.boxObject.width 
+                    - parseInt(document.documentElement.style.width)) / 2);

would be required.
Attached patch Fix dialog centering (obsolete) — Splinter Review
The persisted width is set as an attribute while the default width is set as a style so check them both to get the actual width. Unfortunately neither boxobject or computedstyle can give the actual width at this point since the dialog is not yet visible.
Attachment #223596 - Flags: first-review?
Attachment #223596 - Flags: first-review? → first-review?(mconnor)
Comment on attachment 223596 [details] [diff] [review]
Fix dialog centering


>   // Position the dialog touching the bottom of the toolbox and centered with 
>   // it.
>+  var width = parseInt(document.documentElement.style.width);
>+  if (document.documentElement.hasAttribute("width"))
>+    width = document.documentElement.getAttribute("width");

so, if the if check is true, the line before is useless, make it an if/else.

r+a=me with that fixed
Attachment #223596 - Flags: first-review?(mconnor)
Attachment #223596 - Flags: first-review+
Attachment #223596 - Flags: approval-branch-1.8.1+
Comments addressed, carrying over review and approval.
Attachment #223596 - Attachment is obsolete: true
Attachment #223698 - Flags: first-review+
Attachment #223698 - Flags: approval-branch-1.8.1+
Comment on attachment 223698 [details] [diff] [review]
Centering addressing comments

mozilla/toolkit/content/customizeToolbar.js 	1.29
mozilla/toolkit/content/customizeToolbar.js 	1.26.8.2
> That is correct. The dialog is always positioned just beneath the toolbars to
> be customised when it is displayed. This bug was about stopping it being
> repositioned there every time you added/removed an icon from a toolbar.
Dave,

I've got the latest branch build now and still don't see that this is fixed.  I do see that if I resize the "Toolbar Customization Dialog" the resize persists but centering the dialog does not however persist.

BUILD:Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060529 BonEcho/2.0a3 ID:2006052909

~B

(In reply to comment #21)
> I've got the latest branch build now and still don't see that this is fixed.  I
> do see that if I resize the "Toolbar Customization Dialog" the resize persists
> but centering the dialog does not however persist.

The position of the dialog should not be persisted. That was not the aim of this bug.
Verified.  Thanks for fixing this!
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
*** Bug 302296 has been marked as a duplicate of this bug. ***
*** Bug 279870 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.