Bug 148971 - Node.appendChild(null) / replaceChild(null, null) / removeChild(null) / insertBefore(null, ref) should throw a TypeError
Summary: Node.appendChild(null) / replaceChild(null, null) / removeChild(null) / inser...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://dom.spec.whatwg.org/#node
Keywords: InRadar, WebExposed
Depends on: 148991
Blocks:
  Show dependency treegraph
 
Reported: 2015-09-08 11:18 PDT by Chris Dumez
Modified: 2015-09-10 11:02 PDT (History)
6 users (show)

See Also:


Attachments
WIP Patch (216.92 KB, patch)
2015-09-08 16:50 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (233.24 KB, patch)
2015-09-08 17:08 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (235.58 KB, patch)
2015-09-08 17:26 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (736.07 KB, application/zip)
2015-09-08 18:32 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews103 for mac-mavericks (646.02 KB, application/zip)
2015-09-08 18:38 PDT, Build Bot
no flags Details
Patch (243.89 KB, patch)
2015-09-08 23:17 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (244.59 KB, patch)
2015-09-08 23:35 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (243.13 KB, patch)
2015-09-09 07:57 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (244.14 KB, patch)
2015-09-09 08:25 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (247.16 KB, patch)
2015-09-09 10:09 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (247.06 KB, patch)
2015-09-09 21:49 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (247.34 KB, patch)
2015-09-10 09:25 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (247.29 KB, patch)
2015-09-10 09:44 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2015-09-08 11:18:49 PDT
Node.appendChild(null) / replaceChild(null, null) / removeChild(null) / insertBefore(null, ref) should throw a TypeError:
https://dom.spec.whatwg.org/#node (parameters are not nullable)
Comment 1 Chris Dumez 2015-09-08 11:19:22 PDT
rdar://problem/22560883
Comment 2 Chris Dumez 2015-09-08 11:20:30 PDT
rdar://problem/22559225
Comment 3 Chris Dumez 2015-09-08 16:50:48 PDT
Created attachment 260811 [details]
WIP Patch
Comment 4 WebKit Commit Bot 2015-09-08 16:52:52 PDT
Attachment 260811 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/Element.cpp:324:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 117 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Chris Dumez 2015-09-08 17:08:54 PDT
Created attachment 260814 [details]
Patch
Comment 6 Chris Dumez 2015-09-08 17:26:26 PDT
Created attachment 260818 [details]
Patch
Comment 7 Build Bot 2015-09-08 18:32:11 PDT
Comment on attachment 260818 [details]
Patch

Attachment 260818 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/152475

New failing tests:
js/dom/dot-node-base-exception.html
Comment 8 Build Bot 2015-09-08 18:32:15 PDT
Created attachment 260822 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 9 Build Bot 2015-09-08 18:38:50 PDT
Comment on attachment 260818 [details]
Patch

Attachment 260818 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/152514

New failing tests:
js/dom/dot-node-base-exception.html
fast/events/remove-target-with-shadow-in-drag.html
Comment 10 Build Bot 2015-09-08 18:38:54 PDT
Created attachment 260824 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 11 Chris Dumez 2015-09-08 23:17:14 PDT
Created attachment 260840 [details]
Patch
Comment 12 Chris Dumez 2015-09-08 23:35:41 PDT
Created attachment 260843 [details]
Patch
Comment 13 Chris Dumez 2015-09-09 07:57:07 PDT
Created attachment 260858 [details]
Patch
Comment 14 Chris Dumez 2015-09-09 08:25:39 PDT
Created attachment 260859 [details]
Patch
Comment 15 Chris Dumez 2015-09-09 10:09:26 PDT
Created attachment 260862 [details]
Patch
Comment 16 Chris Dumez 2015-09-09 21:49:15 PDT
Created attachment 260908 [details]
Patch
Comment 17 Ryosuke Niwa 2015-09-10 09:19:17 PDT
Comment on attachment 260908 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260908&action=review

> Source/WebCore/editing/AppendNodeCommand.cpp:38
> -AppendNodeCommand::AppendNodeCommand(PassRefPtr<ContainerNode> parent, PassRefPtr<Node> node, EditAction editingAction)
> +AppendNodeCommand::AppendNodeCommand(PassRefPtr<ContainerNode> parent, Node& node, EditAction editingAction)

I think this should be Ref<Node>.

> Source/WebCore/editing/AppendNodeCommand.h:35
> +    static Ref<AppendNodeCommand> create(PassRefPtr<ContainerNode> parent, Node& node, EditAction editingAction)

Ditto.

> Source/WebCore/editing/AppendNodeCommand.h:41
> -    AppendNodeCommand(PassRefPtr<ContainerNode> parent, PassRefPtr<Node>, EditAction);
> +    AppendNodeCommand(PassRefPtr<ContainerNode> parent, Node&, EditAction);

Ditto.

> Source/WebCore/editing/RemoveNodeCommand.cpp:37
> +RemoveNodeCommand::RemoveNodeCommand(Node& node, ShouldAssumeContentIsAlwaysEditable shouldAssumeContentIsAlwaysEditable)

Ditto.

> Source/WebCore/editing/RemoveNodeCommand.h:35
> -    static Ref<RemoveNodeCommand> create(PassRefPtr<Node> node, ShouldAssumeContentIsAlwaysEditable shouldAssumeContentIsAlwaysEditable)
> +    static Ref<RemoveNodeCommand> create(Node& node, ShouldAssumeContentIsAlwaysEditable shouldAssumeContentIsAlwaysEditable)

Ditto.

> Source/WebCore/editing/RemoveNodeCommand.h:41
> -    explicit RemoveNodeCommand(PassRefPtr<Node>, ShouldAssumeContentIsAlwaysEditable);
> +    RemoveNodeCommand(Node&, ShouldAssumeContentIsAlwaysEditable);

Ditto.

> Source/WebCore/html/track/VTTRegion.h:92
> -    PassRefPtr<HTMLDivElement> getDisplayTree();
> +    HTMLDivElement& getDisplayTree();

We should use Ref<HTMLDivElement> instead.
Comment 18 Chris Dumez 2015-09-10 09:25:01 PDT
Created attachment 260930 [details]
Patch
Comment 19 Chris Dumez 2015-09-10 09:44:41 PDT
Created attachment 260931 [details]
Patch
Comment 20 WebKit Commit Bot 2015-09-10 11:02:27 PDT
Comment on attachment 260931 [details]
Patch

Clearing flags on attachment: 260931

Committed r189576: <http://trac.webkit.org/changeset/189576>
Comment 21 WebKit Commit Bot 2015-09-10 11:02:39 PDT
All reviewed patches have been landed.  Closing bug.