19.93 KB, patch
|Details | Diff | Splinter Review|
New today: run -edit. Bring up Debug->Plaintext Editor. Click anywhere in the text. Get a crash. It seems to be trying to update its bold button, which it doesn't have in the plaintext edit window. Looking at Bonsai for recent changes, I'm guessing this might be related to Hangas' changes, but cc'ing cmanske since he might know why we're trying to update a nonexistant bold button. #0 0x188 in ?? () #1 0x40845e5e in nsXULElement::SetAttribute (this=0x8642ba0, aName=@0xbfffec8c, aValue=@0xbfffecf4) at nsXULElement.cpp:996 #2 0x413b22b8 in nsInterfaceState::SetNodeAttribute (this=0x871e5b0, nodeID=0x413dda39 "Editor:Bold", attributeName=0x413dda34 "bold", newValue=@0xbfffecf4) at nsInterfaceState.cpp:398 #3 0x413b1d9a in nsInterfaceState::UpdateTextState (this=0x871e5b0, tagName=0x413dda45 "b", observerName=0x413dda39 "Editor:Bold", attributeName=0x413dda34 "bold", ioState=@0x871e5c4) at nsInterfaceState.cpp:345 #4 0x413b1158 in nsInterfaceState::ForceUpdate (this=0x871e5b0) at nsInterfaceState.cpp:140 #5 0x413b110b in nsInterfaceState::NotifySelectionChanged (this=0x871e5b0) at nsInterfaceState.cpp:128 #6 0x410827a0 in nsRangeList::NotifySelectionListeners (this=0x8705ff8) at nsRangeList.cpp:1328 #7 0x41081f54 in nsRangeList::TakeFocus (this=0x8705ff8, aNewFocus=0x872b5dc, aContentOffset=313, aContentEndOffset=313, aContinueSelection=0, aMultipleSelection=0) at nsRangeList.cpp:1137 #8 0x410818f2 in nsRangeList::HandleClick (this=0x8705ff8, aNewFocus=0x872b5dc, aContentOffset=313, aContentEndOffset=313, aContinueSelection=0, aMultipleSelection=0, aHint=0) at nsRangeList.cpp:1029
is this linux-only? i am not seeing this problem on win32. also, i doubt that hangas is the best person to be looking at this bug, but maybe i'm missing something?
looks like an XP XUL bug.
taking from hangas
ok, what's happening here is that a broadcaster is being hooked up to a listener node that is contained in an overlay. The overlay is never resolved into the content model, so it's discarded; i.e., the root node of the overlay is released. This causes the listener to be destroyed. Since the broadcaster only has a weak reference to the listener, it is left with a dangling pointer. I think we are too eagerly adding elements from an overlay to the XUL document's element map. We should wait until the overlay is resolved. Hyatt, does this fix sound reasonable to you?
This issues should be fixed, but the editor should suppress the nsInterfaceState's ForceUpdate call when we are editing text, since there's no attributes to update!
okay, the fix for this is getting pretty involved. i've found a bunch of commonalities between nsXULDocument::AddSubtreeToDocument(), ProcessCommonAttributes(), OverlayForwardReference::Merge(), and BroadcasterHookup::Resolve(). I'm refactoring this all into AddSubtreeToDocument(), and just using that instead. I'm worried about causing regressions, and would like to do the dumb thing for M11 (and make editor just not set 'bold' for the plaintext editor), and then land the XUL fix in M12. Does this sound ok?
That's okay with me as long as we can work out a way of figuring out from nsInterfaceState::ForceUpdate() whether we're in a plaintext editor. There doesn't seem to be a way of getting the editor flags from outside nsHTMLEditor. Charley?
Created attachment 2677 [details] [diff] [review] proposed fix (may require dos2unix to apply on unix)
Yes, that patch fixes the problem, and I can use the plaintext editor again, and updating of the status bars still works in the html editor window.
I am seeing some command updating regressions in addressbook, but after talking with hangas, I believe that these are -not- related to this change. I'll go bug chofmann to get this fix in if somebody'll find hyatt and get him to review it. :-)
Waterson's patch for this fixes IM crasher bug 369026
setting sujay as QA contact
fix checked in, r=saari
verified in 12/7 build.
BULK MOVE: Changing component from XUL to XP Toolkit/Widgets: XUL. XUL component will be deleted.