Closed Bug 334450 Opened 15 years ago Closed 14 years ago

nsTextBoxFrame initialization causes incorrect recursion into frame construction

Categories

(Core :: XUL, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: dbaron, Unassigned)

References

Details

When opening the Firefox options dialog in a clean profile, I see the warning added in bug 310985 that there was recursion into frame construction.  The cause is the stack in attachment 218792 [details] (which I incorrectly attached to bug 334430).
I can't reproduce this.  :(  What does |call DumpJSStack()| show when this happesn?
FWIW, it was on a completely clean profile, so it may have something to do with what's inside the default-selected panel (last panel selection is remembered).
Yeah, I tried creating a clean profile; still didn't see this.  :(
Ah, ok.  I've reproduced this now.  I needed to actually enable the assert!  ;)
So the issue here is that the "popup" field of the "autocomplete" binding in autocomplete.xml modifies the anonymous DOM.  So we end up doing that from nsXBLBinding::InstallImplementation, which is called from nsElementSH::PostCreate in this case.

This PostCreate is called when we try to get this.labeledControlElement in the "accessKey" property getter of the "label-control" binding in text.xml.

And that, of course, happens because nsTextBoxFrame::UpdateAttributes calls GetAccessKey on its content node.

I don't see an obvious solution here past doing UpdateAttributes off an event (so NOT inside Init or reflow or anything like that; running script under those is a bad idea) or fixing the autocomplete binding to not modify the DOM from inside binding attachment.

We probably need to do the latter anyway, frankly, otherwise just attaching that binding will assert as far as I can tell.  We have constructors for a reason; this binding should use them.
Flags: blocking1.9a1?
One more note.  Apart from whatever these bindings are doing by accident, a binding could on purpose rip the document apart when we evaluate a field, as things stand.... That's really not good, and probably needs a separate bug. :(
I'd love to see that autocomplete.xml binding fixed :-) However I can't tell whether it should be moved to a constructor or converted to a property.

I'm also interested to know why the popup has to be manually hidden.
bz, mind attaching a callstack of when this happens. It's hard to keep all the calls of comment 5 in my head at the same time.
Comment 0 has the C++ stack.   The JS stack (from DumpJSStack()) only shows the first XBL frame, sadly, not the place where the actual call that caused issues happens.

A pseudo-stack would be:

Re-enter frame construction
nsCSSFrameConstructor::ContentAppended
...
nsXULDocument::ContentAppended
...
nsXULElement::AppendChild
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/widgets/autocomplete.xml&rev=1.57&mark=121#110
nsJSContext::EvaluateStringWithValue
nsXBLProtoImplField::InstallMember
nsXBLProtoImpl::InstallImplementation
nsXBLPrototypeBinding::InstallImplementation
nsXBLBinding::InstallImplementation
nsXBLService::LoadBindings
nsElementSH::PostCreate
Wrap return value of getElementById in an XPCWrappedNative
Call into C++ (getElementById)
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/widgets/text.xml&rev=1.27&mark=255#254
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/widgets/text.xml&rev=1.27&mark=228#224
Get the "accesskey" property from mContent; calls into XBL
nsTextBoxFrame::UpdateAttributes
nsTextBoxFrame::Init
nsCSSFrameConstructor::InitAndRestoreFrame
Normal frame construction, etc.
Flags: blocking1.9a1? → blocking1.9+
Hey folks - any idea of this is still around and relevant?
(In reply to comment #11)
> Looks like this depends on bug 372769
> 

Or more like a dup of Bug 387033. I'll mark this one fixed once the
regression from bug 387033 is fixed.
The warnings I still see when opening preferences are because of bug 335615 and bug 372769. nsTextBoxFrame::Init doesn't cause warnings anymore, because
.accessKey is get in a reflowcallback. Marking this fixed (by bug 387033).
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Duplicate of this bug: 335615
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.