16701 - Editor API Log playback of Insert HRule crashes in DOM code

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[kinmoz]

The following JavaScript causes a crash in nsDOMAttributeMap::SetNamedItem():

    n0 = window.editorShell.editorDocument.createElement("HR");
    a0 = window.editorShell.editorDocument.createAttribute("align");
    a0.value = "left";
    n0.setAttributeNode(a0);

The crash can be recreated by:

  1. Loading a blank page in the editor.
  2. Selecting Debug->StartLog from the menus.
  3. Pressing the "Insert HRule" button on the toolbar.
  4. Selecting Debug->StopLog from the menus.
  5. Selecting Edit->Undo from the menus.
  6. Selecting Debug->RunLog from the menus.

The problem is that attrPrivate is being released twice in SetNamedItem(), at
line 345 and again at line 355. I believe the correct fix for this is to remove
the NS_RELEASE(attrPrivate) call on line 345.

Here's the little snippet of code responsible for the crash:


330                     result =
attribute->QueryInterface(kIDOMAttributePrivateIID,
331                                                        (void
**)&attrPrivate);

332 vidur   1.5         if (NS_SUCCEEDED(result)) {
333                       nsIContent* owner;
334
335                       attrPrivate->GetContent(&owner);
336                       if (owner) {
337                         // The old attribute is already associated with
another
338                         // piece of content.
339                         result = NS_ERROR_DOM_INUSE_ATTRIBUTE_ERR;
340                         NS_RELEASE(owner);
341                       }
342                       else {
343                         attrPrivate->SetContent(mContent);
344                         attrPrivate->SetName(name);
345                         NS_RELEASE(attrPrivate);
346
347                         // Add the new attribute node to the hash table
(maintaining
348                         // a reference to it)
349                         PL_HashTableAdd(attrHash, key, attribute);
350
351                         // Set the attribute on the content
352                         result = mContent->SetAttribute(nameSpaceID,
nameAtom, value, PR_TRUE);
353 vidur   1.5             NS_IF_RELEASE(nameAtom);
354                       }
355 vidur   1.1           NS_RELEASE(attrPrivate);
356                     }

Comment 1

[sujay]

verification steps from Gerardo. Just adding here for when its fixed...


Here is how to verify it, when fixed:

"The crash can be recreated by:

  1. Loading a blank page in the editor.
  2. Selecting Debug->StartLog from the menus.
  3. Pressing the "Insert HRule" button on the toolbar.
  4. Selecting Debug->StopLog from the menus.
  5. Selecting Edit->Undo from the menus.
  6. Selecting Debug->RunLog from the menus."

Comment 2

[kinmoz]

Reassigning bug to [e-mail], I have a fix for it.

Comment 3

[kinmoz]

Checked the following fix into:

     mozilla/layout/base/src/nsDOMAttributeMap.cpp  revision 1.7

Fix was reviewed by [e-mail].


Index: nsDOMAttributeMap.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/base/src/nsDOMAttributeMap.cpp,v
retrieving revision 1.6
diff -c -r1.6 nsDOMAttributeMap.cpp
*** nsDOMAttributeMap.cpp       1999/10/07 00:34:29     1.6
--- nsDOMAttributeMap.cpp       1999/10/25 13:44:40
***************
*** 342,348 ****
            else {
              attrPrivate->SetContent(mContent);
              attrPrivate->SetName(name);
-             NS_RELEASE(attrPrivate);

              // Add the new attribute node to the hash table (maintaining
              // a reference to it)
--- 342,347 ----

Comment 4

[leger]

Adding crash keyword

Comment 5

[Jan Carpenter]

Mass update of qa contact

Comment 6

[Jan Carpenter]

Verified:
2000-09-20-09-M18 : Windows
2000-09-20-08-M18 : Mac
2000-09-20-13-M18 : Linux