Bug 81579 - Refactor ContainerNode::replaceChild to match other mutation methods and share code
Summary: Refactor ContainerNode::replaceChild to match other mutation methods and shar...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Adam Klein
URL:
Keywords:
Depends on: 81683
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-19 16:01 PDT by Adam Klein
Modified: 2019-02-06 09:02 PST (History)
8 users (show)

See Also:


Attachments
Patch (6.30 KB, patch)
2012-03-19 16:07 PDT, Adam Klein
no flags Details | Formatted Diff | Diff
Patch (6.42 KB, patch)
2012-03-20 14:17 PDT, Adam Klein
no flags Details | Formatted Diff | Diff
Patch (6.42 KB, patch)
2012-03-20 14:24 PDT, Adam Klein
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Klein 2012-03-19 16:01:38 PDT
Refactor ContainerNode::replaceChild to match other mutation methods and share code
Comment 1 Adam Klein 2012-03-19 16:07:59 PDT
Created attachment 132697 [details]
Patch
Comment 2 Ojan Vafai 2012-03-19 17:03:34 PDT
Comment on attachment 132697 [details]
Patch

Looks awesome!
Comment 3 WebKit Review Bot 2012-03-19 19:19:31 PDT
Comment on attachment 132697 [details]
Patch

Clearing flags on attachment: 132697

Committed r111310: <http://trac.webkit.org/changeset/111310>
Comment 4 WebKit Review Bot 2012-03-19 19:19:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Csaba Osztrogonác 2012-03-20 02:30:55 PDT
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
Comment 6 Csaba Osztrogonác 2012-03-20 03:45:18 PDT
Skipped on Qt - http://trac.webkit.org/changeset/111378/trunk/LayoutTests/platform/qt/Skipped

Please unskip with the proper fix.
Comment 7 Alexey Proskuryakov 2012-03-20 09:59:08 PDT
Since this caused a regression in core DOM code, rolling out could have been more appropriate.

Adam, do you plan to resolve this soon?
Comment 8 Adam Klein 2012-03-20 10:45:48 PDT
(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
Comment 9 Alexey Proskuryakov 2012-03-20 10:56:02 PDT
Sorry, I don't know anything non-trivial about these tests. The comment in test expectations seems confusing.
Comment 10 Adam Klein 2012-03-20 11:19:50 PDT
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).
Comment 11 Adam Klein 2012-03-20 14:17:20 PDT
Created attachment 132894 [details]
Patch
Comment 12 Adam Klein 2012-03-20 14:24:56 PDT
Created attachment 132898 [details]
Patch
Comment 13 WebKit Review Bot 2012-03-20 15:10:53 PDT
Comment on attachment 132898 [details]
Patch

Clearing flags on attachment: 132898

Committed r111449: <http://trac.webkit.org/changeset/111449>
Comment 14 WebKit Review Bot 2012-03-20 15:10:59 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 mitz 2012-03-20 17:43:19 PDT
(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.
Comment 16 Adam Klein 2012-03-20 17:44:22 PDT
(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.
Comment 17 Adam Klein 2012-03-20 17:48:29 PDT
(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
Comment 18 Lucas Forschler 2019-02-06 09:02:37 PST
Mass moving XML DOM bugs to the "DOM" Component.