Closed Bug 634291 Opened 9 years ago Closed 9 years ago

"ASSERTION: Walking off beginning of list" and crash with navigated-away designMode document

Categories

(Core :: DOM: Editor, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- -

People

(Reporter: jruderman, Assigned: smaug)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, crash, testcase)

Attachments

(3 files)

###!!! ASSERTION: Walking off beginning of list: 'mChild', file layout/base/nsChildIterator.h, line 114

Crash [@ nsCSSFrameConstructor::FindFrameForContentSibling]
Attached file stack traces
Group: core-security
nsHTMLEditor is doing things at unsafe time.
...I think
I don't know the implications of this bug, but noming for blocking to get it on the radar.
blocking2.0: --- → ?
If this gets sg:crit it can block, elsewise: no.
blocking2.0: ? → -
This is a regression from bug 574529.
It is hard to understand the patch for that bug.
Depends on: 574529
Assignee: nobody → Olli.Pettay
Attached patch patchSplinter Review
I think this is what was meant to bug 574529.
Fixes this bug, but I couldn't verify if this regress bug 574529
since that doesn't have a testcase.
Attachment #512809 - Flags: review?(bzbarsky)
Attachment #512809 - Flags: feedback?(dholbert)
Comment on attachment 512809 [details] [diff] [review]
patch

> It is hard to understand the patch for that bug.
The point of that bug was to allow ourselves to flush layout in an SVG-as-an-image document *during* (in service to) its host document's paint.  The patch did that by extending the "safe to flush layout" condition to disregard "IsSafeToRunScript" if scripts are disabled.

> since that doesn't have a testcase.
There's no testcase on that bug because it was a helper-bug for implementing SVG-as-an-image.  No standalone testcase was possible.  I'd suggest running reftests on the "reftests/svg/as-image" directory to test this -- in particular, a locally-viewed copy of layout/reftests/svg/as-image/img-and-image-1.html will work this pretty vigorously.

RE the patch here -- it looks good to me!  I wasn't aware of the distinction between GetScriptGlobalObject vs. GetScriptHandlingObject until now, but from reading their documentation in nsIDocument.h, it looks like we do indeed want the latter here.
Attachment #512809 - Flags: feedback?(dholbert) → feedback+
layout/reftests/svg/as-image/ reftests work.
Comment on attachment 512809 [details] [diff] [review]
patch

r=me
Attachment #512809 - Flags: review?(bzbarsky) → review+
Comment on attachment 512809 [details] [diff] [review]
patch

This should either block or just approved.
Attachment #512809 - Flags: approval2.0?
Comment on attachment 512809 [details] [diff] [review]
patch

a=beltzner
Attachment #512809 - Flags: approval2.0? → approval2.0+
Does not look exploitable from the stack (near 0 crash).
Not that particular case, but I believe there a other cases when we shouldn't be
flushing.
http://hg.mozilla.org/mozilla-central/rev/dc1e5032ed96
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.