Closed Bug 562258 Opened 10 years ago Closed 10 years ago

modal prompts should be made tab-modal

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 59314
Tracking Status
status2.0 --- ?

People

(Reporter: Dolske, Assigned: Dolske)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

[This is bug 59314, but please don't dupe or blocks/depends. It's old and noisy, and I'd like to get keep a high signal-to-noise ratio as this gets started.]

We want to push to reduce the number of modal prompts in the next Firefox release. Modal dialogs suck for all kinds of obvious UI reasons that I won't go into here.

I've started work on a patch that, as a first step, makes alert/confirm/prompt into tab-modal prompts. (Just as notification bars at the moment, that will eventually change.) Next step, if we get this all sorted out, would be to do the same for nsIAuthPrompt2.
Attached patch Patch v.0 (WIP) (obsolete) — Splinter Review
First rough take. Looking for feedback on the general code implementation/design, not the UI.

Haven't addressed any E10S issues here yet, and the needs of people embedding Gecko is just roughly sketched.
How does this kind of approach work with windowed plugins for example.

And note, nsIPromptService is frozen interface, so IMO it should still work
in generic case when embedding gecko.
We can and should break frozen APIs when they no longer make sense, and this is the perfect example. I'd rather remove nsIPromptService completely than try to add shims for existing embedders. Mozilla2 onward!
If we're ok to break APIs, then we need to say that clear and loudly.
In DOM we could and will merge several interfaces asap it is ok to
break frozen APIs.
We're ok to break APIs *if* the gain is sufficient. We don't want to break people without good justification.
(In reply to comment #2)
> How does this kind of approach work with windowed plugins for example.

An interesting question, because I don't understand how plugins would be involved with prompting! What was the issue you had in mind?
You have some (non-native)prompt open somewhere, right?
How does that prevent user to interact with plugins?
We will need to block all user input to the tab (or multiple tabs in the same execution stack).  We might also white out the tab a bit to make it clear that it is now inactive until the prompt is dealt with.  So this would of course also cover any plugins in that tab's content area.

Dolske and I have been talking about the UI for the past few days, and I'll hopefully have some mockups ready soon.
(In reply to comment #7)
> You have some (non-native)prompt open somewhere, right?
> How does that prevent user to interact with plugins?

Ah. I'm not sure yet. We're talking about having the tab-modal UI be something that overlays the whole tab area (<stack>? <panel>? dunno). Presumably that can be made to block interaction with the page because it will simply have a higher z-index than the content. But maybe additional blocking/freezing needs to be done?

[I'm assuming bug 130078 will be fixed to allow this UI, though as a <panel> it might not be needed.]
Depends on: 563114
Depends on: 563274
No longer depends on: 563114
Blocks: 564337
For Firebug, there's a need to have tab-specific panels, so implementing a means of showing and hiding panels per tab (or per window or per application) may be useful enhancement.
Neil: is their any way we could create a tab modal panel that could potentially be dragged outside of the tab?  We are thinking about this for two different cases:

httpauth: how can you really tell that this is a real prompt?  Sites could spoof our current window, but since you can drag it outside of the content area it is clearly real.  Not sure how much I care about this though, and is at best a silly concession to a security UI debate.

javascript alerts/prompts: they could be overlapping content that the user needs to see to respond, so moving the prompt out of the way is reasonably important for some use cases.
Once titlebars on panels are supported, then, yes, moving them would be easy. Also, independently of that, click-and-drag-background moving is possible as well.
Just adding my current list of modal dialogs here:

1. Javascript alerts/prompts, 
2. htauth prompts,
3. master password 

Are there other major categories of things that can lock up a window?
Maybe we should focus this bug on alert/confirm/prompt, which are simple.

the HTTP password and master-password prompts are a little difficult because you may end up with multiple tabs asking for the same auth data, and you have to decide whether to auto-submit the blocked tab B when the user enters the password in blocked tab A.

There's the infamous POSTDATA dialog, which could/should probably just be an error page.
window.showModalDialog() can too, unless that was covered by item nr 1 above.
(In reply to comment #13)
> Just adding my current list of modal dialogs here:
> 
> 1. Javascript alerts/prompts, 

JavaScript slow script/debug dialog, if that's not included in 1.
Is it feasible to have a window-modal dialogue which does not block certain key elements of the user interface? In Mac OS X, for instance, Firefox hides all menus but the application ("Firefox") menu while a modal dialogue is open, even though it doesn't have to (an I would guess it takes more work to make that happen than not). Safari (for Mac), for comparison, leaves all its menus (and sub-menus) visible and enabled, but just disables all its menu items except "Hide Safari", "Hide Others", and "Quit Safari". Safari's "Quit Safari" menu takes effect immediately, despite any modal dialogue being open; it even works if there are multiple windows and/or tabs open (despite being set to warn first in that case -- which it then doesn't.) Firefox also leaves its "Quit Firefox" menu enabled and functional, but it doesn't take effect immediately if there are multiple windows and/or tabs open (and it is set to warn about them) -- in which case it waits until after the modal dialogue is answered to ask the user whether they want to save their tabs, and then quits if they click "Quit". In that way, Firefox's behaviour is closer to what I think the typical user would expect. I think the Quit prompt should displace the modal dialogue (cancelling an alert or making a confirm or prompt or such wait for if the user cancels quitting). Interestingly, Firefox also leaves enabled its "Preferences" menu item, which is also more than Safari does -- and it works too, and on opening the Preferences window, all the menus return and are enabled again. Ideally, all the menus should remain visible and enabled, and specific menu items such as "Close Window", "Close Tab", and perhaps one which might've been added by an extension which specified that said menu item should remain enabled (such as one to do with debugging, if for instance, I would like to have a JavaScript debugger break immediately after the alert I'm seeing):

function f(){
...
a=g();
if(!a) alert("debug: g() didn't return an a."); /* <-- while this alert is still open, I select "Break & Debug Script" from the still-visible-and-enabled "Tools" menu, and after I answer the modal dialogue, I end up in a debugger: */
// <-- here, before h(a) is called, at which point I would loose the information I'm looking for.
h();
...
}
> Are there other major categories of things that can lock up a window?

"Although this page is encrypted, the information you have entered is to be sent over an unencrypted connection and could easily be read by a third party. ... [Continue] [Cancel]" - is window-modal.

Presumably it's part of a family of similar security warnings. I seem to remember a similar one about images; something like "Some elements on this page are not encrypted [Show all]".
Depends on: 569314
Attached patch Patch v.1 (WIP) (obsolete) — Splinter Review
Updated WIP patch.
Attachment #442013 - Attachment is obsolete: true
Blocks: 567804
status2.0: --- → ?
Comment on attachment 448461 [details] [diff] [review]
Patch v.1 (WIP)

>+        let p;
>+        if (this.doModal || !domWin)
>+            p = new ModalPrompter(domWin);
>+        else
>+            p = new TabPrompter(domWin);

In this and all the other cases like it, can the ternary operator not be used to cut down on the excess lines? Seems like that would be cleaner from a code perspective...

let p = (this.doModal || !domWin) ? new ModalPrompter(domWin) : new TabPrompter(domWin);

... unless JS doesn't make that easily possible for some reason.
JS certainly supports ?:, and JS style (C++ but who's counting) favors initializing a declaration with ?: if you can. If there's an if-else chain with fallible calls out of the current function, then not. So yeah, you could write

  let p = (...) ? new ModalPrompter(domWin) : new TabPrompter(domWin);

or even

  let p = new ((...) ? ModalPrompter : TabPrompter)(domWin);

to common the "new" and "(domWin)" parts.

/be
Depends on: 563850
Attached patch Patch v.2 (WIP)Splinter Review
Updated to reflect current changes from bug 563274.
Attachment #448461 - Attachment is obsolete: true
(In reply to comment #13)
window.print()

> Just adding my current list of modal dialogs here:
1. Javascript alerts/prompts, 
2. htauth prompts,
3. master password 
4. JavaScript slow script/debug dialog, Comment 15
5. window.showModalDialog() Comment 16
6. ...sent over an unencrypted connection,  Comment 18
7. Some elements ... not encrypted ,  Comment 18
8. window.print()
(In reply to comment #23)
The JavaScript crypto stuff is modal, and can take a while to run for large keys.

> (In reply to comment #13)
> window.print()
> 
> > Just adding my current list of modal dialogs here:
> 1. Javascript alerts/prompts, 
> 2. htauth prompts,
> 3. master password 
> 4. JavaScript slow script/debug dialog, Comment 15
> 5. window.showModalDialog() Comment 16
> 6. ...sent over an unencrypted connection,  Comment 18
> 7. Some elements ... not encrypted ,  Comment 18
> 8. window.print()
9. crypto.generateCRMFRequest() or form with <keygen> element
Blocks: 583462
Duplicate of this bug: 583462
Prototyping done, moving over to bug 562258 per comment 0.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 59314
(In reply to comment #24)
> > 1. Javascript alerts/prompts, 
> > 2. htauth prompts,
> > 3. master password 
> > 4. JavaScript slow script/debug dialog, Comment 15
> > 5. window.showModalDialog() Comment 16
> > 6. ...sent over an unencrypted connection,  Comment 18
> > 7. Some elements ... not encrypted ,  Comment 18
> > 8. window.print()
> 9. crypto.generateCRMFRequest() or form with <keygen> element
10. POSTDATA dialog, Comment 14
(In reply to comment #27)
> (In reply to comment #24)
> 1. Javascript alerts/prompts, 
> 2. htauth prompts,
> 3. master password 
> 4. JavaScript slow script/debug dialog, Comment 15
> 5. window.showModalDialog() Comment 16
> 6. ...sent over an unencrypted connection,  Comment 18
> 7. Some elements ... not encrypted ,  Comment 18
> 8. window.print()
> 9. crypto.generateCRMFRequest() or form with <keygen> element
> 10. POSTDATA dialog, Comment 14
11. window.onbeforeunload event
> 1. Javascript alerts/confirm/prompts, 
> 2. htauth prompts,
> 3. master password 
> 4. JavaScript slow script/debug dialog, Comment 15
> 5. window.showModalDialog() Comment 16
> 6. ...sent over an unencrypted connection,  Comment 18
> 7. Some elements ... not encrypted ,  Comment 18
> 8. window.print()
> 9. crypto.generateCRMFRequest() or form with <keygen> element
> 10. POSTDATA dialog, Comment 14
> 11. window.onbeforeunload event
12. non-associated protocol dialog, Bug 599662
Blocks: 603901
No longer blocks: 603901
Why is this resolved duplicate of a bug that only fixes one item on the list?

Would it help to create separate bugs for each of these, and list them as blocking this one? (and then reopen this bug)

(I'm only aware of bug 59314 for item #1 and bug 564337 for item #5 so far).
> Why is this resolved duplicate of a bug that only fixes one item on the list?

Because this bug was filed as a duplicate of bug 59314.  See comment 0.

"The list" is just an attempt to hijack the bug.  Please file separate bugs for items on "the list" as needed.
Done, bug 616843 for anyone interested.
You need to log in before you can comment on or make changes to this bug.