Closed
Bug 1019761
Opened 10 years ago
Closed 10 years ago
Firefox crashes when accessing the dialogArguments property of a closed modal Dialog
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla33
People
(Reporter: catalinb, Assigned: catalinb)
References
Details
(Keywords: testcase)
Attachments
(1 file, 2 obsolete files)
2.82 KB,
patch
|
johns
:
review+
|
Details | Diff | Splinter Review |
This bug refers to the deprecated modal dialog implementation. Calling showModalDialog() with a special uri that closes the window and calls dialogArguments() will crash Firefox.
Comment 1•10 years ago
|
||
Minimal testcase attached using "Add an attachment" please.
Keywords: testcase-wanted
Comment 2•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #1) > Minimal testcase attached using "Add an attachment" please. catalin is actively working on this, so testcase and fixes incoming
Status: NEW → ASSIGNED
Keywords: testcase-wanted
Comment 3•10 years ago
|
||
nsGlobalWindow::GetDialogArguments might end up dereferencing a null mDialogArguments, no?
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8433617 [details] [diff] [review] Fix null dialogArguments deref. This patch ensures that an 'undefined' value is returned when dereferencing a null mDialogProperties.
Attachment #8433617 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•10 years ago
|
Summary: Firefox crashes when calling dialogArguments on a closed modal Dialog → Firefox crashes when accessing the dialogArguments property of a closed modal Dialog
Comment 6•10 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=12a8f2c76bb7
Comment 7•10 years ago
|
||
Comment on attachment 8433617 [details] [diff] [review] Fix null dialogArguments deref. r=me
Attachment #8433617 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2eb0ae4b9e79
Flags: in-testsuite+
Keywords: checkin-needed
Comment 9•10 years ago
|
||
Looks like the test fails when run in e10s mode. Backed out. https://hg.mozilla.org/integration/mozilla-inbound/rev/b8e17d034a6f https://tbpl.mozilla.org/php/getParsedLog.php?id=41135810&tree=Mozilla-Inbound
Comment 10•10 years ago
|
||
Also on Android (and probably B2G once they run there)
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8433617 -
Attachment is obsolete: true
Assignee | ||
Comment 12•10 years ago
|
||
Attachment #8436085 -
Attachment is obsolete: true
Assignee | ||
Comment 13•10 years ago
|
||
Try results: https://tbpl.mozilla.org/?tree=Try&rev=b69e5063f7f6
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8436144 [details] [diff] [review] Removed the test from b2g, e10s and android targets. I think the patch is ready for landing. Try results look good - there are two tests failing on Linux x64 debug oth which are not related to my patch. https://tbpl.mozilla.org/?tree=Try&rev=b69e5063f7f6 Please have a look.
Attachment #8436144 -
Flags: review?(jschoenick)
Comment 15•10 years ago
|
||
Comment on attachment 8436144 [details] [diff] [review] Removed the test from b2g, e10s and android targets. Test fix looks good, carrying over r=bz
Attachment #8436144 -
Flags: review?(jschoenick) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1c4d397ba6e4
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•