[Webkit-unassigned] [Bug 17125] Acid3 requires Text.replaceWholeText method from DOM Level 3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 8 00:14:47 PST 2008


darin at apple.com changed:

           What    |Removed                     |Added
  Attachment #18997|review?                     |review-
               Flag|                            |

------- Comment #14 from darin at apple.com  2008-02-08 00:14 PDT -------
(From update of attachment 18997)
I would have preferred to review a combined patch:

+        case Node::CDATA_SECTION_NODE:
+            t = static_cast<const Text*>(n);

The above is incorrect. A CDATA section will be a CharacterData node and *not*
a Text node. So it's an error to cast to Text*. Text is derived from
CharacterData, so if we want to handle both we should be using the type
CharacterData everywhere, and not Text.

I feel bad for spreading the const disease into this code with my suggestion of
marking wholeText() const. Sorry. I probably would have put the const_cast
calls at the call sites inside Text::wholeText and left the helper functions
themselves alone.

+            RefPtr<Node> nodeToRemove(n.release());
             n = n->nextSibling();

This doesn't look like it will work. Calling release() will set n to 0 and
you'll crash on the n->nextSibling() call. You should re-run the tests.

Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

More information about the webkit-unassigned mailing list