Closed
Bug 335615
Opened 18 years ago
Closed 16 years ago
[FIX]Text inputs cause reentry into frame construction ("WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0'")
Categories
(Core :: Layout: Form Controls, defect, P4)
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Blocks 2 open bugs, )
Details
(Whiteboard: [dbaron-1.9:RsCe])
Attachments
(2 files, 1 obsolete file)
(deleted),
text/plain; charset=UTF-8
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
STEPS TO REPRODUCE:
1) Enable assert mentioned in bug 334460
2) Load the testcase URL in this bug
What happens is that once we finish setting up the text control frame we do editor init, while still inside frame construction in general. Editor messes with our anonymous content, causing reentry. Note that bug 217201 made the current setup suck less than it used to, but ideally we'd defer the editor init to after we're all done with frame construction.
Comment 1•18 years ago
|
||
*** Bug 339454 has been marked as a duplicate of this bug. ***
Updated•18 years ago
|
Summary: Text inputs cause reentry into frame construction → Text inputs cause reentry into frame construction ("WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0'")
Updated•18 years ago
|
Flags: blocking1.9?
Updated•18 years ago
|
Flags: blocking1.9? → blocking1.9+
Comment 2•17 years ago
|
||
This bug seems to be a leading cause of console spewage.
Assignee | ||
Comment 3•17 years ago
|
||
Unfortunately, it's kinda hard to fix as things stand. See bug 221820 for my last attempt.
Depends on: 221820
Updated•17 years ago
|
Comment 5•17 years ago
|
||
This is not a dup of bug 334450. That bug is about nsTextBoxFrame, this one is
about nsTextControlFrame
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Comment 6•17 years ago
|
||
Targeting/blocking M9 per request by dbaron.
Target Milestone: --- → mozilla1.9 M9
Comment 7•17 years ago
|
||
Not going to make M9, realistically, but we should really try to get it done pretty soon.
Status: REOPENED → NEW
Target Milestone: mozilla1.9 M9 → ---
Updated•17 years ago
|
Whiteboard: [dbaron-1.9:RsCe]
Comment 8•17 years ago
|
||
I'll try taking a look at this and seeing if there's something I can come up with...
Assignee: nobody → dbaron
Comment 9•17 years ago
|
||
Updated•17 years ago
|
Priority: -- → P4
Comment 10•17 years ago
|
||
Time to drop this one from the blocker list.
Flags: blocking1.9+ → wanted1.9+
Flags: wanted1.9-
Flags: wanted1.9+
Flags: wanted-next+
Updated•16 years ago
|
Flags: wanted1.9.1?
Flags: wanted1.9.1? → wanted1.9.1+
Assignee | ||
Comment 11•16 years ago
|
||
This basically takes a large chunk of the patch in bug 221820, but NOT the actual lazy init of the editor. Instead, we set up a bunch of our things (like our text listener, etc) under CreateAnonymousContent, and then put off editor init (a combination of the current InitEditor and the current CreateFrameFor) to a script runner. That will make sure that it runs as soon as the scriptblocker (which should be in place, since we're in frame construction!) gets removed.
A lot of the patch is just moving code around without changing it, but it could still use a somewhat careful read. I haven't found any issues with the new ordering, but this is somewhat fragile code. :(
Assignee: dbaron → bzbarsky
Status: NEW → ASSIGNED
Attachment #357427 -
Flags: superreview?(roc)
Attachment #357427 -
Flags: review?(mats.palmgren)
Assignee | ||
Updated•16 years ago
|
Summary: Text inputs cause reentry into frame construction ("WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0'") → [FIX]Text inputs cause reentry into frame construction ("WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0'")
Assignee | ||
Updated•16 years ago
|
Attachment #357427 -
Flags: superreview?(roc) → superreview+
Comment 12•16 years ago
|
||
Comment on attachment 357427 [details] [diff] [review]
Proposed fix
r=mats
> layout/forms/nsTextControlFrame.cpp
You can just remove nsTextControlFrame::CreateFrameFor() since
"return nsnull" is the default behaviour.
> nsTextControlFrame::AttributeChanged
> {
> if (!mEditor || !mSelCon)
>- return NS_ERROR_NOT_INITIALIZED;
>+ return NS_OK;
Why not "return nsStackFrame::AttributeChanged(...)" ?
> layout/forms/nsTextControlFrame.h
s/NSIGFXTEXTCONTROLFRAME2/NSITEXTCONTROLFRAME/g
>+ void DelayedEditorInit();
I'd prefer if it were non-public.
> layout/generic/nsIAnonymousContentCreator.h
You can remove "class nsPresContext;" while you're here.
Also, this comment seems out of date:
> HTML frames like nsFileControlFrame currently use this as well as XUL frames
> like nsScrollbarFrame and nsSliderFrame.
Neither nsScrollbarFrame/nsSliderFrame use it AFAICT.
Attachment #357427 -
Flags: review?(mats.palmgren) → review+
Assignee | ||
Comment 13•16 years ago
|
||
> You can just remove nsTextControlFrame::CreateFrameFor()
Good catch. Done.
> Why not "return nsStackFrame::AttributeChanged(...)" ?
We never call that, actually, and even the call to nsBoxFrame looks weird... I'll make this call nsBoxFrame, though. Should be safe enough.
> s/NSIGFXTEXTCONTROLFRAME2/NSITEXTCONTROLFRAME/g
Done.
> I'd prefer if it were non-public.
OK. Moved to a friend class, I guess.
> Also, this comment seems out of date:
Fixed.
Attachment #357427 -
Attachment is obsolete: true
Assignee | ||
Comment 14•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 17 years ago → 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Target Milestone: --- → mozilla1.9.2a1
You need to log in
before you can comment on or make changes to this bug.
Description
•