Closed Bug 611566 Opened 10 years ago Closed 10 years ago

Remove Enter/LeaveModalState calls from nsGlobalWindow alert/confirm/prompt


(Core :: DOM: Core & HTML, defect)

Not set





(Reporter: Dolske, Assigned: Dolske)




(1 file)

Bug 61098 added calls to Enter/LeaveModalState in the alert/confirm/prompt methods in nsGlobalWindow. They're technically not needed (since they call prompt service, and code within that call ensures the modal state is updated), but are basically harmless because the EMS/LMS code can handle multiple calls.

I happened to ask jst about this, he said these were just added as a convenience for testing.

But now I have need to remove them. Bug 59314 is adding tab-modal dialogs, and the latest patch revision is making nested calls [main() --> page A alert() --> page B alert()] work better by having all the dialog closing code run when the dialog's button is clicked, instead of waiting for the it's event loop to notice the dialog is done. EG, in the example above clicking on page A's alert() should do something without waiting for a response to page B's alert().

Alas, because of these modal state calls in nsGlobalWindow, the content remains frozen until the event loop spins and the prompt service returns.
Attached patch Patch v.1Splinter Review
The core of this patch is the obvious change to nsGlobalWindow.cpp, and the addition of _toggleModalState() to test_bug61098.html.

I also refactored the mock prompt stuff in the test, while I was there, because bug 59314 will need it to implement nsIPromptFactory. Basically the existing test code gets moved into a MockPrompt object (which is tied to a specific DOM window), and the MockPromptService just creates one of these on demand and calls it... This is basically how the existing code in nsPrompter.js works.
Attachment #491024 - Flags: review?(jst)
Attachment #491024 - Flags: review?(jst) → review+
blocking2.0: --- → betaN+
blocking2.0: betaN+ → ---
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.