Bug 81579 – Refactor ContainerNode::replaceChild to match other mutation methods and share code

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Adam Klein]

Refactor ContainerNode::replaceChild to match other mutation methods and share code

[Adam Klein]

Created attachment 132697 [details]
Patch

[Ojan Vafai]

Comment on attachment 132697 [details]
Patch

Looks awesome!

[WebKit Review Bot]

Comment on attachment 132697 [details]
Patch

Clearing flags on attachment: 132697

Committed r111310: <http://trac.webkit.org/changeset/111310>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

Reopen, because it broke dom/xhtml/level3/core/nodereplacechild30.xhtml on GTK and on Qt:

--- /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/dom/xhtml/level3/core/nodereplacechild30-expected.txt 
+++ /home/webkitbuildbot/slaves/release32bit-NRWT/buildslave/qt-linux-32-release-NRWT/build/layout-test-results/dom/xhtml/level3/core/nodereplacechild30-actual.txt 
@@ -1,2 +1,3 @@
 Test	http://www.w3.org/2001/DOM-Test-Suite/level3/core/nodereplacechild30
-Status	Success
+Status	error
+Message	Error: NOT_FOUND_ERR: DOM Exception 8

[Csaba Osztrogonác]

Skipped on Qt - http://trac.webkit.org/changeset/111378/trunk/LayoutTests/platform/qt/Skipped

Please unskip with the proper fix.

[Alexey Proskuryakov]

Since this caused a regression in core DOM code, rolling out could have been more appropriate.

Adam, do you plan to resolve this soon?

[Adam Klein]

(In reply to comment #7)
> Since this caused a regression in core DOM code, rolling out could have been more appropriate.
> 
> Adam, do you plan to resolve this soon?

I'm looking into this now. Do you know much about these tests? The Chromium test expectations ignore them with this comment:

// XHTML tests.  These tests seem like they work, but only because the
// expected output expects to see JS errors.  There is no point in running
// these tests, because they are giving us a false sense of testing that isn't
// really happening.  Furthermore, since they appear to pass if we do try to
// run them, we can't even list them as permanently expected to fail.
WONTFIX SKIP : dom/xhtml = PASS

[Alexey Proskuryakov]

Sorry, I don't know anything non-trivial about these tests. The comment in test expectations seems confusing.

[Adam Klein]

Rolled out in http://trac.webkit.org/changeset/111415, somewhat mysterious.

And I'd like to figure out why we're not running those tests on Chromium, since that one anyway seems to run fine (at least, it did before my change).

[Adam Klein]

Created attachment 132894 [details]
Patch

[Adam Klein]

Created attachment 132898 [details]
Patch

[WebKit Review Bot]

Comment on attachment 132898 [details]
Patch

Clearing flags on attachment: 132898

Committed r111449: <http://trac.webkit.org/changeset/111449>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

(In reply to comment #13)
> (From update of attachment 132898 [details])
> Clearing flags on attachment: 132898
> 
> Committed r111449: <http://trac.webkit.org/changeset/111449>

The code changes in r111449 are identical to those in r111310, so it’s no wonder that they caused dom/xhtml/level3/core/nodereplacechild30.xhtml to fail again.

[Adam Klein]

(In reply to comment #15)
> (In reply to comment #13)
> > (From update of attachment 132898 [details] [details])
> > Clearing flags on attachment: 132898
> > 
> > Committed r111449: <http://trac.webkit.org/changeset/111449>
> 
> The code changes in r111449 are identical to those in r111310, so it’s no wonder that they caused dom/xhtml/level3/core/nodereplacechild30.xhtml to fail again.

Arg, git fail. Fix coming.

[Adam Klein]

(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #13)
> > > (From update of attachment 132898 [details] [details] [details])
> > > Clearing flags on attachment: 132898
> > > 
> > > Committed r111449: <http://trac.webkit.org/changeset/111449>
> > 
> > The code changes in r111449 are identical to those in r111310, so it’s no wonder that they caused dom/xhtml/level3/core/nodereplacechild30.xhtml to fail again.
> 
> Arg, git fail. Fix coming.

Committed http://trac.webkit.org/changeset/111478

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.