Closed
Bug 564652
Opened 14 years ago
Closed 14 years ago
"ASSERTION: We shouldn't be reentering here" involving scrollbar reflow and editor
Categories
(Core :: Layout, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla2.0b4
People
(Reporter: jruderman, Assigned: bzbarsky)
References
Details
(Keywords: assertion)
Attachments
(2 files, 2 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
dbaron
:
review+
dbaron
:
approval2.0+
|
Details | Diff | Splinter Review |
###!!! ASSERTION: We shouldn't be reentering here: '!mFrameIsUpdatingScrollbar', file /home/jruderman/mozilla-central/layout/generic/nsGfxScrollFrame.cpp, line 2981
I couldn't figure out how to reproduce this assertion, but the stack trace looks useful. Other bugs suspicious of nsGfxScrollFrameInner::SetCoordAttribute include bug 373564 (which smaug fixed in 2007), bug 437537, and bug 338674.
Comment 1•14 years ago
|
||
This looks related to bug 471166 to me.
One way to fix this would be to set a script blocker in nsGfxScrollFrameInner::SetCoordAttribute, but I wonder if a better fix would be to always run EditingStateChanged off of a script runner?
This just happened on Tinderbox.
Assignee | ||
Comment 3•14 years ago
|
||
OK, looks like this blocks relanding bug 577607.
I think I've convinced myself that the editor thing is the only issue here.
What's happening is that when we get our contenteditable element initially, we bail out of nsHTMLDocument::DeferredContentEditableCountChange because mParser is non-null. In EndLoad we null out the parser and call EditingStateChanged, but HasPresShell() returns false for the window because we haven't done the async frame init in the parent yet as far as I can tell. So we skip out of EditingStateChanged and leave a guerilla editor init lying around to run the next time an EndUpdate happens to come along while we have a presshell.
Assignee | ||
Comment 4•14 years ago
|
||
Attachment #465075 -
Flags: review?(roc)
Attachment #465075 -
Flags: review?(roc) → review+
Attachment #465075 -
Flags: approval2.0+
Assignee | ||
Comment 5•14 years ago
|
||
I tested the patch some, and realized that it can cause crashes because CreateShell is not called inside a scriptblocker (and that in general, document viewer init is not all that robust).
So I modified it such that if MaybeEditingStateChanged modifies the mPresShell of the document from whatever doCreateShell set up we bail out. I think that's the safe thing to do for now, pending more sanity in editor-land and document viewer.
Attachment #465075 -
Attachment is obsolete: true
Assignee | ||
Comment 6•14 years ago
|
||
Except that double-deletes the styleset... We really need a scriptblocker here.
Assignee | ||
Comment 7•14 years ago
|
||
For what it's worth, I tried a scriptblocker. Then we crash in nsFrameLoader::Show because showing the viewer makes its mDocShell null when running docshell/base/crashtests/432114-1.html.
Then I tried null-checking that and end up with nasty docloader and necko asserts.
Assignee | ||
Comment 8•14 years ago
|
||
Attachment #465292 -
Attachment is obsolete: true
Attachment #465348 -
Flags: review?(roc)
Attachment #465348 -
Flags: approval2.0?
Comment 9•14 years ago
|
||
Comment on attachment 465348 [details] [diff] [review]
OK, this seems like what I want to do here
r=dbaron
Attachment #465348 -
Flags: review?(roc)
Attachment #465348 -
Flags: review+
Attachment #465348 -
Flags: approval2.0?
Attachment #465348 -
Flags: approval2.0+
Comment 10•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → bzbarsky
Flags: in-testsuite+
Priority: -- → P1
Target Milestone: --- → mozilla2.0b4
You need to log in
before you can comment on or make changes to this bug.
Description
•