Won't prevent the page from additional dialogs though the option is selected if the window.prompt is cancelled




6 years ago
6 years ago


(Reporter: Makoto Mizukami, Assigned: emk)


Firefox Tracking Flags

(Not tracked)



(3 attachments, 1 obsolete attachment)



6 years ago
Created attachment 596270 [details]
Testcase of this issue

User Agent: Opera/9.80 (X11; Linux x86_64; U; en-GB) Presto/2.10.229 Version/11.61

Steps to reproduce:

1. Open the testcase attached. This testcase shows one window.alert, one window.confirm, one window.prompt and another window.prompt in this order.
2. When the first window.prompt appears, switch "Prevent the page from additional daialogs" on and choose "Cancel".

Actual results:

Another window.prompt appears.

Expected results:

Further dialogs should not appear.

Note that this behaviour is reasonably expected since it goes so with window.confirm. You can confirm it with the testcase by selecting the option and choosing Cancel in the window.confirm.

Comment 1

6 years ago
I note here that I first heard about this issue from Yosuke Hasegawa ( http://utf-8.jp/ ).
Attachment #596270 - Attachment mime type: text/plain → text/html
Choosing 'Prevent this page from creating additional dialogs' option in the first window.prompt and clicking cancel prevents any additional dialog boxes. Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:13.0a1) Gecko/20120210 Firefox/13.0a1 and 10.0 on Windows. Possibly a Linux only bug.

Comment 3

6 years ago
Hmm... It is reproducible with my Mozilla/5.0 (Windows NT 5.1; rv:13.0a1) Gecko/20120210 Firefox/13.0a1 ...
I can confirm comment#2: wfm with Mozilla/5.0 (Windows NT 6.1; rv:13.0a1) Gecko/20120208 Firefox/13.0a1 SeaMonkey/2.10a1 and FF10

Comment 5

6 years ago
I couldn't reproduce the problem either on:
Mozilla/5.0 (Windows NT 5.1; rv:13.0a1) Gecko/20120212 Firefox/13.0a1
Mozilla/5.0 (Windows NT 5.1; rv:10.0) Gecko/20100101 Firefox/10.0
Could try with Safe mode and/or clean profile?

Comment 6

6 years ago
Ah, I got it.
It need to check at the third prompt (not second).

Comment 7

6 years ago
Created attachment 596603 [details]
Less confusing test case

It was reproducible even without the confirm() prompt.


6 years ago
Component: Untriaged → DOM
Ever confirmed: true
OS: Linux → All
Product: Firefox → Core
QA Contact: untriaged → general
Hardware: x86_64 → All
Version: 10 Branch → unspecified

Comment 8

6 years ago
Problem code was in the toolkit.
Component: DOM → General
Product: Core → Toolkit
QA Contact: general → general

Comment 9

6 years ago
Created attachment 596679 [details] [diff] [review]
Assignee: nobody → VYV03354
Attachment #596679 - Flags: review?(gavin.sharp)
This seems like a reasonable change, but it has broad impact.

I can't really think of any ways in which someone could depend on the outparam being unmodified in the "clicked cancel" case... I suppose callers might have set a default value on the parameter and not expect it to be clobbered in the cancel case?

There certainly is potential value in propagating that to the caller. Seems like we should be consistent with the "checkValue" and "value", though.

Comment 11

6 years ago
Comment on attachment 596679 [details] [diff] [review]

/tests/toolkit/components/prompts/test/test_modal_prompts.html needs to be fixed as well.
Attachment #596679 - Flags: review?(gavin.sharp)

Comment 12

6 years ago
Created attachment 597376 [details] [diff] [review]
patch with test fix

Try result is here:
Attachment #596679 - Attachment is obsolete: true
Attachment #597376 - Flags: review?(gavin.sharp)
Comment on attachment 597376 [details] [diff] [review]
patch with test fix

On further thought, I'm not sure that it's a good idea to change this behavior. It looks like we pretty intentionally decided to pick this behavior given the test (and IIRC we tried to match the previous longstanding implementation's behavior exactly). This bug as filed doesn't seem like a serious enough problem to be revisiting this decision (the entire "don't show more dialogs" UI is kind of janky, maybe we'll end up improving that at some point). Would be interested in hearing dolske's thoughts, though.
Attachment #597376 - Flags: review?(gavin.sharp) → feedback?(dolske)

This mostly worked pre-4.0, but the old code was rather inconstant with what it returned upon "cancel", so I tried to clean it up such that when the user cancels the dialog there was no state passed back to the caller. This is a weird case where chrome is effectively overloading the purpose of a content dialog; even though the user is saying "cancel!" to content it's assuming the user is also saying "ok! no more!" to chrome.

And generally "cancel" means to ignore/skip whatever's being asked for. Consider a hypothetical UI like:

    |                                              |
    | Thanks for shopping with us! Are you ready   |
    | to complete this order and change your       |
    | credit card?                                 |
    |                                              |
    | [x] Add me to your mailing list.             |
    |                                              |
    |                              [Cancel]  [Ok]  |

What should clicking cancel here do? I'd be surprised if it added me to their mailing list!

I suspect the behavior this bug about only makes sense to us because we know what's actually happening in the browser. Also, this would have been a more tricky problem to solve in the past when these dialogs were not content-modal, and could be easily confused with a real system dialog... Consider an annoyance script with the old dialogs doing |while(true) { confirm('format hard drive?') }|. :-)

So, I also think we should probably not make the change in this patch.

Aside: I think this was already broken in the old code but no one noticed!


Sure looks like for prompt(), the checkbox value is only returned to the caller if Ok is clicked. Oops. :)
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
Attachment #597376 - Flags: feedback?(dolske) → feedback-
Thanks for tracking this down, though. I at least hope you found the current prompt code not nearly as terrible as the old code. ;)
You need to log in before you can comment on or make changes to this bug.