445070 - Weave Sign-In Dialog won't go away until I close all "Help" windows that it spawned

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Daniel Holbert [:dholbert]]

Steps to reproduce:
 1. Sign out of Weave (if you're signed in).
 2. Click Weave icon in status bar, and choose "Sign In".
 3. Click "Help" on the sign-in dialog.
    --> This brings up a new browser window pointed to a help URL (which is currently "Not Found" -- bug 445068).
    ** Don't close this new "Help" browser window. **
 4. Click "Sign In" on the sign-in dialog.

ACTUAL RESULTS:
  Sign in dialog remains open, after you're signed in, **until you close the new "Help" browser window.**
  -- Clicking 'Cancel' does nothing.
  -- Clicking 'Help' again brings up *another* browser window, which must also be closed before the dialog will dismiss itself.
  -- Pressing 'Esc' does nothing.

  The only way I can find to close the dialog (aside from closing its "Help" browser window(s)) is to click the "x" close button on the window's title bar.

This bug is particularly nasty because:
  - The Weave sign-in dialog is modal -- i.e. it blocks all interaction with the window behind it.  So, it really sucks when it won't go away.
  - The spawned "Help" window looks just like any other browser window, which makes it hard for the user to tell that the sign-in dialog's behavior depends on this window.  This is particularly true when there's no actual weave help content present in the window, as is the current situation. :) (bug 445068)

VERSION INFO:
Ubuntu Linux 8.04
Firefox 3.0.0
Weave 0.2.4

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot of issue

Comment 2

[bomfog]

Same on windows, OS -> ALL

Comment 3

[Daniel Holbert [:dholbert]]

I still see this bug in Weave 0.2.98.  Version --> Trunk

Comment 4

[Mike Connor [:mconnor]]

Opening a window from a modal dialog/window will cause that window to be modal.

bug 425419 is a similar bug in Firefox.

Comment 5

[Anant Narayanan [:anant]]

http://hg.mozilla.org/labs/weave/rev/4e8afad961ae

Comment 6

[Mike Connor [:mconnor]]

is _openWin going to be a public method on Utils then?

Comment 7

[Ed Lee :Mardak]

There is a Utils.openWindow method that wraps Utils._openWin. Not sure why that wasn't used.

Comment 8

[Ed Lee :Mardak]

Ah. You want to open a http uri instead of chrome uri. I suppose openWindow could detect if it should prepend chrome content or just use the full uri.

Comment 9

[Mike Connor [:mconnor]]

or add an openContentWindow method for non-chrome windows.  Either way, we shouldn't call private methods from other components.

This is the type of thing that code review would catch, please don't bypass the process...

Comment 10

[Anant Narayanan [:anant]]

Attachment: Re-structure window utils

Re-structure window utils to remove usage of private methods.

Comment 11

[Mike Connor [:mconnor]]

Comment on attachment 391029 [details] [diff] [review]
Re-structure window utils

If you're using it to open chrome windows, _openChromeWindow would be correct naming.

Comment 12

[Anant Narayanan [:anant]]

http://hg.mozilla.org/labs/weave/rev/5e639c1f6ebe