Closed
Bug 611566
Opened 14 years ago
Closed 14 years ago
Remove Enter/LeaveModalState calls from nsGlobalWindow alert/confirm/prompt
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla2.0b8
People
(Reporter: Dolske, Assigned: Dolske)
References
Details
Attachments
(1 file)
10.69 KB,
patch
|
jst
:
review+
Gavin
:
approval2.0+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•14 years ago
|
||
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)
Updated•14 years ago
|
Attachment #491024 -
Flags: review?(jst) → review+
Updated•14 years ago
|
blocking2.0: --- → betaN+
Updated•14 years ago
|
Attachment #491024 -
Flags: approval2.0+
Assignee | ||
Comment 2•14 years ago
|
||
Status: NEW → RESOLVED
blocking2.0: betaN+ → ---
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•