Closed Bug 334450 Opened 17 years ago Closed 16 years ago
Text Box Frame initialization causes incorrect recursion into frame construction
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.
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.
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: 16 years ago
Resolution: --- → FIXED
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.