Closed
Bug 608191
Opened 14 years ago
Closed 13 years ago
"ASSERTION: EnsureReflowFlushAndPaint() called with no docshell!" with prompt()
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b12
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: jruderman, Assigned: Natch)
Details
(Keywords: assertion, testcase)
Attachments
(2 files)
47 bytes,
text/html
|
Details | |
1.25 KB,
patch
|
bzbarsky
:
review+
jst
:
approval2.0+
|
Details | Diff | Splinter Review |
###!!! ASSERTION: EnsureReflowFlushAndPaint() called with no docshell!: 'mDocShell', file dom/base/nsGlobalWindow.cpp, line 4298
Reporter | ||
Comment 1•14 years ago
|
||
nsGlobalWindow::EnsureReflowFlushAndPaint [dom/base/nsGlobalWindow.cpp:4300] nsGlobalWindow::Prompt [dom/base/nsGlobalWindow.cpp:4584] NS_InvokeByIndex_P ...
Comment 2•14 years ago
|
||
Uh.... that assert is bogus, no? Or the callsite needs to be checking for a docshell.
Assignee | ||
Comment 3•14 years ago
|
||
(In reply to comment #2) > Uh.... that assert is bogus, no? Or the callsite needs to be checking for a > docshell. Yes, it's bogus. See bug 623447. We bail anyhow the same way in ::GetTop, where we anyhow check if there is no docshell. These need to be fixed.
Assignee | ||
Comment 4•14 years ago
|
||
(In reply to comment #3) > (In reply to comment #2) > > Uh.... that assert is bogus, no? Or the callsite needs to be checking for a > > docshell. > > Yes, it's bogus. See bug 623447. We bail anyhow the same way in ::GetTop, where > we anyhow check if there is no docshell. These need to be fixed. Wait, scratch that. I'm not sure I understand this. The docshell goes away after the call to GetTop? Is this still happening?
Assignee | ||
Comment 5•14 years ago
|
||
Ok, so I'm thinking this might just be due to the fact that AreDialogsBlocked isn't forwarded to the outer window (as isn't Prompt). So there's probably a docshell on the outer window but not on the inner. Does that make any sense?
Assignee | ||
Comment 6•14 years ago
|
||
(Oh, and GetTop is forwarded, so there's no check on mDocShell for the inner window).
Comment 7•14 years ago
|
||
> The docshell goes away after the call to GetTop? What call to GetTop? > So there's probably a docshell on the outer window but not on the inner. This statement is correct.
Assignee | ||
Comment 8•14 years ago
|
||
(In reply to comment #7) > > The docshell goes away after the call to GetTop? > > What call to GetTop? In Prompt there's a call to AreDialogsBlocked. In AreDialogsBlocked there is a call to GetTop, which checks if there is a docshell. If there isn't we return null and AreDialogsBlocked returns true which should cause us to bail with an error from Prompt. > > So there's probably a docshell on the outer window but not on the inner. > > This statement is correct. Then that seems to be the problem. EnsureReflowFlushAndPaint is not forwarded. In Alert and Confirm that means it's acting on the outer window since Alert and Confirm are both forwarded. With Prompt that means it's acting on the inner window. It won't get caught in GetTop since GetTop is forwarded.
Assignee | ||
Comment 9•14 years ago
|
||
Also, I can't see how this wouldn't happen on 1.9.2, can someone confirm if it happens there as well?
Comment 10•14 years ago
|
||
Sounds like we should forward Prompt (and maybe EnsureReflowFlushAndPaint, or just add an assert in it that it's running on the outer window).
Assignee | ||
Comment 11•14 years ago
|
||
Had to add the forwarding to ShowModalDialog too.
Assignee: nobody → highmind63
Attachment #502023 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 12•14 years ago
|
||
This assertion was added in bug 115352 when the function was changed from accepting a docshell as a parameter to using mDochShell. I'm not sure if this is a serious assertion in this case, we don't crash in a release build because we check for mDocShell anyhow and return early. However, that means that we don't flush layout before going into modal mode. Again, in this particular case, I don't know if that's bad. To clarify this, I'm requesting blocking.
blocking2.0: --- → ?
Comment 13•14 years ago
|
||
Not doing this particular flush is, iirc, a spoofing vector.
Assignee | ||
Comment 14•14 years ago
|
||
This goes too far back for me, the EnsureReflowFlushAndPaint originated in bug 77002. What kind of spoofing is possible? AFAICT the worry is that some layout values won't be correct (e.g. .width and .height).
Comment 15•14 years ago
|
||
Making it look like the prompt is from a different website from the one it's actually from, by posing it during paint suppression.
Assignee | ||
Comment 16•14 years ago
|
||
(In reply to comment #15) > Making it look like the prompt is from a different website from the one it's > actually from, by posing it during paint suppression. I don't see how that can be done with this testcase, however it can be easily done with onunload="prompt()" and the user closing the tab...
Comment 18•13 years ago
|
||
Comment on attachment 502023 [details] [diff] [review] like so? Imo, yes.
Attachment #502023 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•13 years ago
|
Attachment #502023 -
Flags: approval2.0?
Updated•13 years ago
|
Attachment #502023 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 19•13 years ago
|
||
Pushed: http://hg.mozilla.org/mozilla-central/rev/47cbe0ba759c
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
Assignee | ||
Updated•13 years ago
|
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
•