Closed Bug 403942 Opened 17 years ago Closed 16 years ago

Cancelling "add new toolbar" *after* closing customize panel will freeze tab switching functionality

Categories

(Toolkit :: Toolbars and Toolbar Customization, defect, P1)

All
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla1.9beta4

People

(Reporter: tchung, Assigned: stanshebs)

References

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.8) Gecko/20071004 Firefox/2.0.0.8
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.8) Gecko/20071004 Firefox/2.0.0.8

If you cancel the "Add new Toolbar" action on customizing toolbar, it will freeze the browser from navigating to other tabs.  Need to force quit the browser and restart to resume your work.


Reproducible: Always

Steps to Reproduce:
1. Launch Fx3 beta 1 rc3.  Open up a few new tabs.
2. right click toolbar > customize...
3. Click "Add new Toolbar" button
4. Click done on the customize toolbar
5. Then click "Cancel" for New Toolbar window
6. Verify you are unable to switch to any of your open tabs on the browser.

Browser is now in a hosed state.  Need to Force > Quit to restore your work.

Note: this seems to occur after Bug 403557 is executed.   perhaps its related?
Actual Results:  
Frozen tab switching.

Expected Results:  
Able to move around tabs
Forces user to hard restart.  
Depends on: 403557
Flags: blocking-firefox3?
Priority: -- → P2
Not terribly surprising that you get into a bad state: your steps involve closing the parent of a modal .prompt() before closing the child. But it won't be possible to tell whether it's a Widget: Cocoa bug that it allows you to interact with the parent while the prompt is up until bug 395334 keeps the panel from getting on top of everything on earth.
Assignee: nobody → joshmoz
Component: Toolbars → Widget: Cocoa
Depends on: 395334
No longer depends on: 403557
Flags: blocking-firefox3?
Priority: P2 → --
Product: Firefox → Core
QA Contact: toolbars → cocoa
Summary: Cancelling "add new toolbar" will freeze tab switching functionality → Cancelling "add new toolbar" *after* closing customize panel will freeze tab switching functionality
Yes, this is fixed by the patch in bug 395334.
Neil: I'm still not sure whose bug it really is, but it's *worse* after the landing from bug 395334 - now, you open the panel, open the sheet from the panel, click the Done button in the panel, and the whole window hides... somewhere, I'm not sure where. Cmd+tab to get it back, and you're back to step 5 in the original STR.
Flags: blocking1.9?
Version: unspecified → Trunk
Marking this as a dupe per comment #3, the window hiding in comment #4 is bug 406342.
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: blocking1.9?
Resolution: --- → DUPLICATE
Um, no. Comment 3 was completely incorrect: bug 395334 did not fix this, it just introduced another step where your window is not only broken, but also hidden.

Bug 406342 may or may not also fix this, at the same time that it fixes that window hiding, which would make this "fixed by bug 406342" not "duplicate of", but it certainly isn't a duplicate of bug 395334.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
And just to be as unobscure as possible, it was *not* fixed by bug 406342, it's still possible to interact with the customization panel (in every way, though only clicking Done makes the browser unusable) while the add toolbar sheet is up and in theory being modal.
Status: REOPENED → NEW
Assignee: joshmoz → stanshebs
Bug is still happening in today's trunk. It doesn't seem to interfere with the creation and use of new windows, or use of menu or toolbar in the multiple-tab window - everything just goes to the last tab. Quitting doesn't work while the multiple-tab window is up, but you can close that window and then quitting works normally, so at least force quits aren't necessary. It looks like simple loss of mouse focus for the window, with no way to get it back.


This sounds like a problem with sheet modality (since both the
Customize dialog and the New Toolbar dialog are sheets).

The New Toolbar dialog should be modal in such a way as to prevent
anything from happening when the user clicks on the Customize dialog's
Done button.  This is how it works on the 1.8 branch (tested in
Firefox 2.0.0.11).

So this bug seems related to bug 410170, and this behavior may (like
that described at bug 410170) may end up getting changed by the fix
for bug 395465 (which may end up changing all the sheets into ordinary
modal dialogs/windows).
Hardware: Macintosh → All
In the absence of a general solution for sheets that want to be modal, I'm looking at a smaller fix, which is simply to tear down the New Toolbar dialog (which is an nsPromptService) when the Customize dialog is exited.

(In reply to comment #9)
> This sounds like a problem with sheet modality (since both the
> Customize dialog and the New Toolbar dialog are sheets).
> 

The customize dialog isn't a sheet, it's a popup with the window as a parent.

Attached patch patch to disable done button (obsolete) (deleted) — Splinter Review
Here is a simple patch that simply disables the "Done" button while the add new toolbar dialog is up. Not as powerful a solution as changing around window types at a low level, but since that work is still in progress, this solves the immediate problem.
Sounds like a good idea to me.

I hope to get to the larger problem in time for beta 4.
Attachment #296778 - Flags: review?(mano)
Comment on attachment 296778 [details] [diff] [review]
patch to disable done button

1. As for the workaround, this will break both Fx (on windows and linux) and Thunderbird (everywhere) as you didn't add the same id to customizeToolbar.xul
2. So the issue is that the modal feature is simply broken when used within a noautohide-panel, are you sure you're willing to ship this as-is?
Attachment #296778 - Flags: review?(mano) → review-
Attached patch improved patch to disable done button (obsolete) (deleted) — Splinter Review
OK, this version addresses the portability issue.

And yes, this is not the ideal solution, but our timing is such that the necessary low-level work may not even get done for Firefox 3, so we need to do something else. This at least prevents the user from getting wedged in a nasty way.
Attachment #296778 - Attachment is obsolete: true
Attachment #298567 - Flags: review?(mano)
Comment on attachment 298567 [details] [diff] [review]
improved patch to disable done button

>Index: toolkit/content/customizeToolbar.js
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/content/customizeToolbar.js,v
>retrieving revision 1.40
>diff -u -8 -p -r1.40 customizeToolbar.js
>--- toolkit/content/customizeToolbar.js	15 Nov 2007 03:38:16 -0000	1.40
>+++ toolkit/content/customizeToolbar.js	22 Jan 2008 23:45:48 -0000
>@@ -512,20 +512,25 @@ function addNewToolbar()
>                                 .getService(Components.interfaces.nsIPromptService);
> 
>   var stringBundle = document.getElementById("stringBundle");
>   var message = stringBundle.getString("enterToolbarName");
>   var title = stringBundle.getString("enterToolbarTitle");
> 
>   var name = {};
> 
>+  var doneButton = document.getElementById("donebutton");
>+  doneButton.disabled = true;
>+

a comment referring to this bug would be nice...

r=mano.  Would you file a bug for your backend work?
Attachment #298567 - Flags: review?(mano) → review+
I think the patch I'm working on for bug 395465 has got this problem
fixed (at a lower level than Stan's patch).

It still needs some testing, but I hope to post it tomorrow.
Attached patch Patch with added comment (deleted) — Splinter Review
OK, a version reflecting feedback and ready for checkin.
Attachment #298567 - Attachment is obsolete: true
Keywords: checkin-needed
Priority: P3 → P1
Status: NEW → ASSIGNED
Component: Widget: Cocoa → Toolbars and Toolbar Customization
Product: Core → Toolkit
QA Contact: cocoa → toolbars
Checking in browser/base/content/customizeToolbarSheet.xul;
/cvsroot/mozilla/browser/base/content/customizeToolbarSheet.xul,v  <--  customizeToolbarSheet.xul
new revision: 1.7; previous revision: 1.6
done
Checking in toolkit/content/customizeToolbar.js;
/cvsroot/mozilla/toolkit/content/customizeToolbar.js,v  <--  customizeToolbar.js
new revision: 1.42; previous revision: 1.41
done
Checking in toolkit/content/customizeToolbar.xul;
/cvsroot/mozilla/toolkit/content/customizeToolbar.xul,v  <--  customizeToolbar.xul
new revision: 1.21; previous revision: 1.20
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta4
Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko Minefield/3.0b4pre ID:2008020623
Status: RESOLVED → VERIFIED
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: