Bug 148971

Summary:

Node.appendChild(null) / replaceChild(null, null) / removeChild(null) / insertBefore(null, ref) should throw a TypeError

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

DOM

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, buildbot, commit-queue, kling, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar, WebExposed

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

URL:

https://dom.spec.whatwg.org/#node

Bug Depends on:

148991

Bug Blocks:

Attachments:

Description	Flags
WIP Patch	none
Patch	none
Patch	none
Archive of layout-test-results from ews104 for mac-mavericks-wk2	none
Archive of layout-test-results from ews103 for mac-mavericks	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

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.