Closed Bug 334515 Opened 18 years ago Closed 18 years ago

crash initialising iframe as html edit where html loaded contains a second iframe [@ nsQueryInterface::operator()]

Categories

(Core :: DOM: Editor, defect)

Product:
Core
Component:
DOM: Editor
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: ntmott, Assigned: sicking)

References

Details

(5 keywords, Whiteboard: [camino-1.0.1])

Crash Data

Attachments

(6 files, 4 obsolete files)

updated test case
765 bytes, text/html
Details
Patch to fix
5.34 KB, patch
dbaron
: review+
dbaron
: superreview+
dbaron
: approval-branch-1.8.1+
Details | Diff | Splinter Review
Minimal patch for the 1.8.0 branch
5.28 KB, patch
dveditz
: approval1.8.0.3+
dveditz
: approval1.8.0.4+
Details | Diff | Splinter Review
Unregress editing v2
5.59 KB, patch
Details | Diff | Splinter Review
Unregress editing, branch version
5.52 KB, patch
dveditz
: approval-branch-1.8.1+
dveditz
: approval1.8.0.3+
dveditz
: approval1.8.0.4+
Details | Diff | Splinter Review
Full combined patch
6.25 KB, patch
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2

Setting an iframe as an html edit frame and loading html content which contains a second iframe crashes Firefox 1.5.0.2 (have not tried other versions)

The content seems to load, but then the iframe content disappears. Clicking on the iframe then causes Firefox to crash.



Reproducible: Always

Steps to Reproduce:
1. Create a html page with the following code:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<title>Untitled Document</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<script>
function initIframe(){
var html;
html = "<html><head></head><body>Hello World!<br><iframe src=\"http://www.mozilla.org\">&nbsp;</iframe></body></html>";
var RTE_example = document.getElementById("example");
RTE_example.contentWindow.document.designMode = "on";
RTE_example.contentWindow.document.open();
RTE_example.contentWindow.document.write(html);
RTE_example.contentWindow.document.close();
};
</script>
</head>
<body onLoad="javascript: initIframe();">
<iframe id="example"><html><head></head><body>&nbsp;</body></html></iframe>
</body>
</html>

2. load the page
3. click on the iframe

Actual Results:  
the expected content is not displayed and Firefox crahses.

Expected Results:  
an editable iframe should have been produced.
Attached file reporter's testcase (obsolete) — 

    
    

    
  

      
      
      
      

        Attached file
          
        updated test case
        — 
      

    


  
The previous test test missed a space between 'iframe' and 'src'.

    
    

    
  

      
      
      
      

        Attached file
          
        details of the crash (obsolete)
        — 
      

    


  

    
    

    
  
Could you please install Talkback and get a Talkback ID for the crash? http://kb.mozillazine.org/Talkback

    
    

    
  
No need for talkback id.

xpcom_core.dll!nsQueryInterface::operator()(const nsID & aIID={...}, void * * answer=0x0012c368)  Line 47
editor.dll!nsCOMPtr<nsIEditor>::assign_from_qi(nsQueryInterface qi={...}, const nsID & aIID={...})  Line 1232
editor.dll!nsCOMPtr<nsIEditor>::nsCOMPtr<nsIEditor>(nsQueryInterface qi={...})  Line 646
editor.dll!nsUndoCommand::IsCommandEnabled(const char * aCommandName=0x04663148, nsISupports * aCommandRefCon=0x0483afe0, int * outCmdEnabled=0x0012c564)  Line 74
embedcomponents.dll!nsControllerCommandTable::IsCommandEnabled(const char * aCommandName=0x04663148, nsISupports * aCommandRefCon=0x0483afe0, int * aResult=0x0012c564)  Line 138
embedcomponents.dll!nsBaseCommandController::IsCommandEnabled(const char * aCommand=0x04663148, int * aResult=0x0012c564)  Line 118
xpcom_core.dll!XPTC_InvokeByIndex(nsISupports * that=0x00000003, unsigned int methodIndex=2, unsigned int paramCount=1230164, nsXPTCVariant * params=0x00000000)  Line 102
xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=3)  Line 2155
xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=CALL_METHOD)  Line 2155
xpc3250.dll!XPC_WN_CallMethod(JSContext * cx=0x03b47308, JSObject * obj=0x04442aa0, unsigned int argc=1, long * argv=0x048cda84, long * vp=0x0012c818)  Line 1444
js3250.dll!js_Invoke(JSContext * cx=0x03b47308, unsigned int argc=1, unsigned int flags=0)  Line 1246
js3250.dll!js_Interpret(JSContext * cx=0x03b47308, unsigned char * pc=0x03a87760, long * result=0x0012d2cc)  Line 3902
js3250.dll!js_Invoke(JSContext * cx=0x03b47308, unsigned int argc=1, unsigned int flags=2)  Line 1270
js3250.dll!js_InternalInvoke(JSContext * cx=0x03b47308, JSObject * obj=0x044ce850, long fval=73556936, unsigned int flags=0, unsigned int argc=1, long * argv=0x0012d558, long * rval=0x0012d560)  Line 1347
js3250.dll!JS_CallFunctionValue(JSContext * cx=0x03b47308, JSObject * obj=0x044ce850, long fval=73556936, unsigned int argc=1, long * argv=0x0012d558, long * rval=0x0012d560)  Line 4186
gklayout.dll!nsJSContext::CallEventHandler(JSObject * aTarget=0x044ce850, JSObject * aHandler=0x046263c8, unsigned int argc=1, long * argv=0x0012d558, long * rval=0x0012d560)  Line 1426
gklayout.dll!nsJSEventListener::HandleEvent(nsIDOMEvent * aEvent=0x0485a690)  Line 186
gklayout.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct * aListenerStruct=0x0440ba00, nsIDOMEventListener * aListener=0x0440b958, nsIDOMEvent * aDOMEvent=0x0485a690, nsISupports * aCurrentTarget=0x0440b8a0, unsigned int aSubType=32, unsigned int aPhaseFlags=6)  Line 1636
gklayout.dll!nsEventListenerManager::HandleEvent(nsPresContext * aPresContext=0x00cd06e0, nsEvent * aEvent=0x0012d9c4, nsIDOMEvent * * aDOMEvent=0x0012d77c, nsISupports * aCurrentTarget=0x0440b8a0, unsigned int aFlags=6, nsEventStatus * aEventStatus=0x0012d780)  Line 1740
gklayout.dll!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6)  Line 335
gklayout.dll!nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6, nsDispatchingCallback * aCallback=0x00000000)  Line 455
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 401
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventDispatcher::Dispatch(nsISupports * aTarget=0x0440b8a0, nsPresContext * aPresContext=0x00cd06e0, nsEvent * aEvent=0x0012d9c4, nsIDOMEvent * aDOMEvent=0x00000000, nsEventStatus * aEventStatus=0x0012d9c0, nsDispatchingCallback * aCallback=0x00000000, int aTargetIsChromeHandler=0)  Line 575
gklayout.dll!nsXULCommandDispatcher::UpdateCommands(const nsAString_internal & aEventName={...})  Line 415
gklayout.dll!nsGlobalWindow::UpdateCommands(const nsAString_internal & anAction={...})  Line 4472
gklayout.dll!nsFocusController::UpdateCommands()  Line 217
gklayout.dll!nsFocusController::Focus(nsIDOMEvent * aEvent=0x04637cf0)  Line 362
gklayout.dll!DispatchToInterface(nsIDOMEvent * aEvent=0x04637cf0, nsIDOMEventListener * aListener=0x03b4714c, unsigned int (nsIDOMEvent *)* aMethod=0x0187ce50, const nsID & aIID={...}, int * aHasInterface=0x0012dc34)  Line 143
gklayout.dll!nsEventListenerManager::HandleEvent(nsPresContext * aPresContext=0x04892528, nsEvent * aEvent=0x0012e388, nsIDOMEvent * * aDOMEvent=0x0012dd18, nsISupports * aCurrentTarget=0x03a83454, unsigned int aFlags=4, nsEventStatus * aEventStatus=0x0012dd1c)  Line 1730
gklayout.dll!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=4)  Line 335
gklayout.dll!nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6, nsDispatchingCallback * aCallback=0x00000000)  Line 429
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 401
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventDispatcher::Dispatch(nsISupports * aTarget=0x048d14f8, nsPresContext * aPresContext=0x04892528, nsEvent * aEvent=0x0012e388, nsIDOMEvent * aDOMEvent=0x00000000, nsEventStatus * aEventStatus=0x0012e384, nsDispatchingCallback * aCallback=0x00000000, int aTargetIsChromeHandler=0)  Line 575
gklayout.dll!nsEventStateManager::PreHandleEvent(nsPresContext * aPresContext=0x04892528, nsEvent * aEvent=0x0012e6f8, nsIFrame * aTargetFrame=0x049376bc, nsEventStatus * aStatus=0x0012e50c, nsIView * aView=0x03b58228)  Line 639
gklayout.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0012e6f8, nsIView * aView=0x03b58228, nsEventStatus * aStatus=0x0012e50c)  Line 6120
gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x03b58228, nsGUIEvent * aEvent=0x0012e6f8, nsEventStatus * aEventStatus=0x0012e50c)  Line 5901
gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x03b58228, nsPoint aPoint={...}, nsGUIEvent * aEvent=0x0012e6f8, int aCaptured=0)  Line 1712
gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0012e6f8, nsEventStatus * aStatus=0x0012e634)  Line 1665
gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012e6f8)  Line 174
gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012e6f8, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1100
gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012e6f8)  Line 1121
gkwidget.dll!nsWindow::DispatchFocus(unsigned int aEventType=105, int isMozWindowTakingFocus=1)  Line 6215
gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=7, unsigned int wParam=4981478, long lParam=0, long * aRetValue=0x0012eaf8)  Line 4752
gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x002f0566, unsigned int msg=7, unsigned int wParam=4981478, long lParam=0)  Line 1289
user32.dll!77d48734()
[Frames below may be incorrect and/or missing, no symbols loaded for user32.dll]
user32.dll!77d48816()
user32.dll!77d4b4c0()
user32.dll!77d4b50c()
ntdll.dll!7c90eae3()
user32.dll!77d4da6c()
gkwidget.dll!nsWindow::SetFocus(int aRaise=1)  Line 2156
gklayout.dll!nsEventStateManager::SendFocusBlur(nsPresContext * aPresContext=0x04892528, nsIContent * aContent=0x00000000, int aEnsureWindowHasFocus=1)  Line 4291
gklayout.dll!nsEventStateManager::SetContentState(nsIContent * aContent=0x00000000, int aState=2)  Line 3877
gklayout.dll!nsEventStateManager::PostHandleEvent(nsPresContext * aPresContext=0x04892528, nsEvent * aEvent=0x0012f654, nsIFrame * aTargetFrame=0x04950874, nsEventStatus * aStatus=0x0012f418, nsIView * aView=0x03b58228)  Line 1944
gklayout.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0012f654, nsIView * aView=0x03b58228, nsEventStatus * aStatus=0x0012f418)  Line 6148
gklayout.dll!PresShell::HandlePositionedEvent(nsIView * aView=0x03b58228, nsIFrame * aTargetFrame=0x04950874, nsGUIEvent * aEvent=0x0012f654, nsEventStatus * aEventStatus=0x0012f418)  Line 6005
gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x047eb720, nsGUIEvent * aEvent=0x0012f654, nsEventStatus * aEventStatus=0x0012f418)  Line 5826
gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x047eb720, nsPoint aPoint={...}, nsGUIEvent * aEvent=0x0012f654, int aCaptured=0)  Line 1712
gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0012f654, nsEventStatus * aStatus=0x0012f540)  Line 1665
gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012f654)  Line 174
gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012f654, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1100
gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012f654)  Line 1121
gkwidget.dll!nsWindow::DispatchMouseEvent(unsigned int aEventType=302, unsigned int wParam=1, long lParam=393222)  Line 6095
gkwidget.dll!ChildWindow::DispatchMouseEvent(unsigned int aEventType=302, unsigned int wParam=1, long lParam=393222)  Line 6278
gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=513, unsigned int wParam=1, long lParam=393222, long * aRetValue=0x0012fac0)  Line 4568
gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x001f0630, unsigned int msg=513, unsigned int wParam=1, long lParam=393222)  Line 1289
user32.dll!77d48734()
user32.dll!77d48816()
user32.dll!77d489cd()
user32.dll!77d49402()
user32.dll!77d48a10()
gkwidget.dll!nsAppShell::Run()  Line 135
tkitcmps.dll!nsAppStartup::Run()  Line 161
xul.dll!XRE_main(int argc=3, char * * argv=0x00ac7458, const nsXREAppData * aAppData=0x004036b0)  Line 2364
firefox.exe!main(int argc=3, char * * argv=0x00ac7458)  Line 61
firefox.exe!__tmainCRTStartup()  Line 586
firefox.exe!mainCRTStartup()  Line 403
kernel32.dll!7c816d4f()
kernel32.dll!7c8399f3()

Keywords: crash
Summary: Firefox crashes when initialising iframe as html edit where html loaded contains a second iframe → Firefox crashes when initialising iframe as html edit where html loaded contains a second iframe [@ nsQueryInterface::operator()]
Version: unspecified → Trunk

    
    

    
  
The closest bug I could find was bug 299676.

Confirming using a trunk build from ~16-th.
Assignee: nobody → mozeditor
Status: UNCONFIRMED → NEW
Component: General → Editor
Ever confirmed: true
Keywords: testcase
Product: Firefox → Core
QA Contact: general
Summary: Firefox crashes when initialising iframe as html edit where html loaded contains a second iframe [@ nsQueryInterface::operator()] → crash initialising iframe as html edit where html loaded contains a second iframe [@ nsQueryInterface::operator()]

    
  

        Attachment #218865 -
        Attachment is obsolete: true

    
  

        Attachment #218873 -
        Attachment is obsolete: true
Flags: blocking1.9a1?

    
  
Assignee: mozeditor → bugmail

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch to fix
        — 
          — Splinter Review
      

    


  
This fixes the crash. I no longer crash from the testcases in this bug, bug 331981 and bug 211348.

I still see very strange behaviour from the testcase in bug 331981, but at least the security issue seems to be plugged.

        Attachment #219692 -
        Flags: superreview?(bzbarsky)

        Attachment #219692 -
        Flags: review?(bzbarsky)

        Attachment #219692 -
        Flags: superreview?(bzbarsky)

        Attachment #219692 -
        Flags: superreview+

        Attachment #219692 -
        Flags: review?(bzbarsky)

        Attachment #219692 -
        Flags: review+

    
    

    
  
Comment on attachment 219692 [details] [diff] [review]
Patch to fix

sr=me, fwiw.  Thanks for doing this!

        Attachment #219692 -
        Flags: approval1.8.0.3?

    
    

    
  
So this patch should be very safe to land with a minimal risk of regression. The patch basically makes us use safe mechanisms for avoiding access to deleted objects, rather than relying on that users clear out the pointer manually.

The only downside to the patch is that not all objects can be used as context, only objects that supports weak references. However all the objects that we use today already do so it should be no problem for existing code.

I havn't investigated yet how exploitable this crash is. I'll do that tomorrow.
Flags: blocking1.8.0.3?

    
    

    
  


  
The trunk patch didn't apply cleanly to the 1.8 branch because bug 332132 recently landed on trunk, fixing an OOM crash and changing some whitespace. This patch applies cleanly and I went ahead and added the null checks from 332132.

        Attachment #219775 -
        Flags: review?(bugmail)

        Attachment #219775 -
        Flags: approval-branch-1.8.1?(bzbarsky)

        Attachment #219775 -
        Flags: approval-branch-1.8.1?(bzbarsky) → approval-branch-1.8.1?(dbaron)

        Attachment #219775 -
        Flags: approval-branch-1.8.1?(dbaron) → approval-branch-1.8.1+

    
    

    
  
Comment on attachment 219775 [details] [diff] [review]
1.8 branch patch (includes bug 332132)

Btw, this is already landed on the 1.8 branch. Still needs to go on the 1.8.0.3 branch

        Attachment #219775 -
        Flags: review?(bugmail)

        Attachment #219775 -
        Flags: review+

        Attachment #219775 -
        Flags: approval1.8.0.3?

        Attachment #219775 -
        Flags: approval-branch-1.8.1+

        Attachment #219692 -
        Flags: approval1.8.0.3?

    
    

    
  


  
This is a minimal patch for the 1.8.0 branch that only includes the parts neccesary to fix the crash.

(This is also what I landed yesterday on the 1.8.1 branch)
Flags: blocking1.9a1?

    
    

    
  
Comment on attachment 219799 [details] [diff] [review]
Minimal patch for the 1.8.0 branch

approved for the 1.8.0 branch, a=dveditz for drivers.

        Attachment #219799 -
        Flags: approval1.8.0.3+

    
    

    
  
Comment on attachment 219799 [details] [diff] [review]
Minimal patch for the 1.8.0 branch

Does this patch still fix the crash in  bugs that the current bug depends on: bug
331981 and bug 211348.  See comment #7 regarding attachment 219692 [details] [diff] [review]

    
    

    
  
So I'm beginning to wonder if any extensions could depend on this object -- it's got a contractID, so they *could* be using it, which means this *could* break them.

Any thoughts on what, if anything, we should do about that?


For the suite, according to sicking, we apparently also need to do stuff for the editor XBL binding, since editor elements are passed as context by the editor code.
Flags: blocking1.8.0.4?
Flags: blocking1.8.0.4+
Flags: blocking1.8.0.3+

    
    

    
  
Comment on attachment 219799 [details] [diff] [review]
Minimal patch for the 1.8.0 branch

approved for 1.8.0.3 mini-branch

        Attachment #219799 -
        Flags: approval1.8.0.3+

    
    

    
  
nsBaseCommandController is not a pseudo-interface: we aren't worrying about changes to concrete classes, just interfaces and pseudo-interfaces such as nsIDocument/nsIContent

    
    

    
  
I've said this in person to everyone involved, but for the record:

* We are incompatibly changing an API to require nsISupportsWeakReference be implemented by the nsISupports * doodad.

* That fixes a potentially exploitable security bug, good.  Alternative of holding a strong ref looks like a leak hazard.

* An API is a contract, which requires all of our code to comply (or be sued!).

* We therefore ought to take the second patch that Jonas has developed, to fix the editor XBL to comply with the new API.

/be

    
    

    
  
It is a relatively important semantic change to nsIControllerContext, even though the signature is unchanged.

    
    

    
  
Speaking of which, we should probably document that change in nsIControllerContext.idl.

    
    

    
  
I have a patch that changes nsIControllerContext.idl that I used to verify that all internal callers were ok. I'll file a new bug on that and attach the patch there.


I am worried about extensions too, but at the time I don't see another solution than the current one. Unfortunatly the code in question is too unowned :(

    
    

    
  
Just thought I should note this does not affect Firefox 1.0.x or Suite 1.7.x: Martijn Wargers notes in bug 331981 comment 0:

   This is a regression.
   It doesn't crash in a 2005-04-05 build, it does crash in a 2005-04-06 build,
   so my bet is on bug 283897.

That's a year after the aviary/moz1.7 branches, and I've confirmed that Firefox 1.0.8 is not affected.

    
    

    
  
So one thing we could do wrt extensions is that if the nsISupports passed in doesn't support nsISupportsWeakReference then we store an nsISupports* like we used to.  This does mean that extensions that use this stuff will suffer from issues like this bug if we fail to unset contexts, of course....

The reason bug 283897 was likely relevant here is that it changed editor's behavior when doing a load in an iframe which is in designMode.  Maybe we're not clearing contexts in that case properly for some reason?  Say we try to work with a window's controllers after it's already dropped them or something?

    
    

    
  
No crash in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3 with attachment 218871 [details]

    
    

    
  
The patch in attachment 219775 [details] [diff] [review] also fixes a crash in my company's Pearl Comments extension.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Unregress editing (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #219951 -
        Flags: superreview?(dbaron)

        Attachment #219951 -
        Flags: review?(dbaron)

        Attachment #219951 -
        Flags: approval-branch-1.8.1?(dbaron)

    
    

    
  
Comment on attachment 219951 [details] [diff] [review]
Unregress editing

You should initialize mCommandContextRawPtr to nsnull in the constructor, and maybe propagate errors from GetWeakReference.  With that, r+sr=dbaron.

        Attachment #219951 -
        Flags: superreview?(dbaron)

        Attachment #219951 -
        Flags: superreview+

        Attachment #219951 -
        Flags: review?(dbaron)

        Attachment #219951 -
        Flags: review+

        Attachment #219951 -
        Flags: approval-branch-1.8.1?(dbaron)

        Attachment #219951 -
        Flags: approval-branch-1.8.1+

    
    

    
  


  
Fixes review comments

        Attachment #219951 -
        Attachment is obsolete: true

    
    

    
  


  
This one applies to the 1.8.* branches

        Attachment #219954 -
        Flags: approval1.8.0.3?

    
    

    
  


  
This is the full combined patch for distributors and private trees and the like.

    
    

    
  
Comment on attachment 219953 [details] [diff] [review]
Unregress editing v2

post facto a=dveditz for Firefox 1.5.0.3 minibranch, 1.8.0 branch, and 1.8 branch

        Attachment #219953 -
        Flags: approval1.8.0.4+

        Attachment #219953 -
        Flags: approval1.8.0.3+

        Attachment #219953 -
        Flags: approval-branch-1.8.1+

    
    

    
  
Comment on attachment 219953 [details] [diff] [review]
Unregress editing v2

oops, wrong one.

        Attachment #219953 -
        Flags: approval1.8.0.4+

        Attachment #219953 -
        Flags: approval1.8.0.3+

        Attachment #219953 -
        Flags: approval-branch-1.8.1+

    
    

    
  
Comment on attachment 219954 [details] [diff] [review]
Unregress editing, branch version

a=dveditz for branch landing (for real this time)

        Attachment #219954 -
        Flags: approval1.8.0.4+

        Attachment #219954 -
        Flags: approval1.8.0.3?

        Attachment #219954 -
        Flags: approval1.8.0.3+

        Attachment #219954 -
        Flags: approval-branch-1.8.1+

    
    

    
  
Comment on attachment 219775 [details] [diff] [review]
1.8 branch patch (includes bug 332132)

ignore this one, handle bug 332132 elsewhere

        Attachment #219775 -
        Attachment is obsolete: true

    
  
Whiteboard: [camino-1.0.1]

    
    

    
  
Marking this bug fixed. I'll file separate bugs on remaining work to do on the trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Blocks: 299676

    
    

    
  
Verified FIXED using 04-27-16 build of SeaMonkey trunk on Windows XP.
Status: RESOLVED → VERIFIED

    
    

    
  
This bug has the same root cause as the Firefox DoS that hit the press this past week. Dupe-finding fodder:

MozillaZine:      http://forums.mozillazine.org/viewtopic.php?t=408603
Vuln PoC home:    http://www.securident.com/vuln/ff.txt
Bugtraq posting:  http://www.securityfocus.com/archive/1/431878/30/0/
Secunia:          http://secunia.com/advisories/19802/


    
    

    
  
Just updated to 1.5.0.3

Firefox no longer crashes, but the iframe still does not function properly. 

The contents of the outer iframe still do not load correctly. I should be able to edit text inside around the embedded iframe.

Nick 
 

    
    

    
  
Nick, could you please file a followup bug on that?  Thanks!

    
    

    
  
the Problem is the function document.close()
if an iframe is in the written html (maybe it closes that content and gif back a editable reference to that)
if the html is later added everthing works fine

    
    

    
  
Robert (Nick?), please file follow-up bugs for the issues you're seeing (preferably with a testcase), and mention the bug number here.
Blocks: 337040

    
    

    
  
for "document.close()" with written iframe an behavior discussed
the new created bug is: bug 337040 
(with testcase and description)
Crash Signature: [@ nsQueryInterface::operator()]

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: