Skip to content
Permalink
Browse files
Range API should throw a TypeError for null Node parameters 
https://bugs.webkit.org/show_bug.cgi?id=148692

Reviewed by Ryosuke Niwa.

Source/WebCore:

Range API should throw a TypeError for null Node parameters. We currently
throw a NotFoundError.

As per the DOM specification, the Node arguments are not optional nor
nullable [1]:
https://dom.spec.whatwg.org/#range

Therefore, as per the Web IDL specification, we should throw a TypeError
if the Node parameter is null or missing:
https://heycam.github.io/webidl/#es-interface (step 1).

[1] https://heycam.github.io/webidl/#es-nullable-type

No new tests, covered by existing tests which have been
rebaselined.

* dom/Range.cpp:
(WebCore::Range::setStart):
(WebCore::Range::setEnd):
(WebCore::Range::isPointInRange):
(WebCore::Range::comparePoint):
(WebCore::Range::compareNode):
(WebCore::Range::compareBoundaryPoints):
(WebCore::Range::intersectsNode):
(WebCore::Range::insertNode):
(WebCore::Range::setStartAfter):
(WebCore::Range::setEndBefore):
(WebCore::Range::setEndAfter):
(WebCore::Range::selectNode):
(WebCore::Range::selectNodeContents):
(WebCore::Range::surroundContents):
(WebCore::Range::setStartBefore):
Set the Exception code to TypeError instead of NOT_FOUND_ERR if
the Node parameter is null.

* dom/Range.idl:
Stop marking the Node parameters as optional. They are not optional in
the specification and they are not really optional in our implementation.
Previously, if the Node parameter was missing, we would call the
implementation with a null pointer and the implementation would throw a
NotFoundError. Now that they are mandatory, the bindings will directly
throw a TypeError (as per the Web IDL spec) if the Node parameter is
missing. However, if the JavaScript explicitely passes null or undefined,
the implementation will still be called with a null pointer (because
our bindings generator does not distinguish nullable / non-nullable
parameters). For this reason, we still need to handle null pointers on
the implementation side.

LayoutTests:

Update / rebaseline existing tests.

* fast/dom/Range/range-compareNode.html:
* fast/dom/Range/range-intersectsNode-expected.txt:
* fast/text/text-combine-crash-expected.txt:
* http/tests/w3c/dom/interfaces-expected.txt:
* http/tests/w3c/dom/ranges/Range-comparePoint-2-expected.txt:
* http/tests/w3c/dom/ranges/Range-intersectsNode-binding-expected.txt:


Canonical link: https://commits.webkit.org/166818@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@189240 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
@cdumez
cdumez committed Sep 2, 2015
1 parent cdb6baa commit 9432db4446fcd63e64c72ab907293f0456f34abd
Showing 10 changed files with 157 additions and 131 deletions.
16 LayoutTests/ChangeLog
View file
@@ -1,3 +1,19 @@
2015-09-01 Chris Dumez <cdumez@apple.com>

Range API should throw a TypeError for null Node parameters
https://bugs.webkit.org/show_bug.cgi?id=148692

Reviewed by Ryosuke Niwa.

Update / rebaseline existing tests.

* fast/dom/Range/range-compareNode.html:
* fast/dom/Range/range-intersectsNode-expected.txt:
* fast/text/text-combine-crash-expected.txt:
* http/tests/w3c/dom/interfaces-expected.txt:
* http/tests/w3c/dom/ranges/Range-comparePoint-2-expected.txt:
* http/tests/w3c/dom/ranges/Range-intersectsNode-binding-expected.txt:

2015-09-01 Jeremy Jones <jeremyj@apple.com>

Unreviewed, Layout Test http/tests/inspector/dom/disconnect-dom-tree-after-main-frame-navigation.html is failing
2 LayoutTests/fast/dom/Range/range-compareNode.html
View file
@@ -182,7 +182,7 @@
try {
result = range.compareNode(node);
} catch (e) {
if(e.code == DOMException.NOT_FOUND_ERR) {
if(e instanceof TypeError) {
document.getElementById("test19").innerHTML = "test 19 passed: deleted node";
} else {
document.getElementById("test19").innerHTML = "<span style=\"color: red;\">test 19 failed error: " + e.message + "</span>";
2 LayoutTests/fast/dom/Range/range-intersectsNode-expected.txt
View file
@@ -59,7 +59,7 @@ PASS range.selectNode(document) threw exception Error: InvalidNodeTypeError: DOM
PASS intersects is false

2.6 Node deleted
PASS range.intersectsNode(node) threw exception Error: NotFoundError: DOM Exception 8.
PASS range.intersectsNode(node) threw exception TypeError: Type error.

PASS successfullyParsed is true

6 LayoutTests/fast/text/text-combine-crash-expected.txt
View file
@@ -6,12 +6,12 @@ foo 


Errlog webtest_fn_1: TypeError: undefined is not an object (evaluating 'document.applets[0].addEventListener')
Errlog webtest_fn_2: NotFoundError: NotFoundError: DOM Exception 8
Errlog webtest_fn_2: TypeError: Type error
Errlog webtest_fn_3: TypeError: undefined is not an object (evaluating 'document.images[2].contentEditable="true"')
Errlog webtest_fn_8: TypeError: null is not an object (evaluating 'lis.length')
Errlog webtest_fn_9: TypeError: undefined is not an object (evaluating 'document.anchors[4].setAttribute')
Errlog webtest_fn_10: NotFoundError: NotFoundError: DOM Exception 8
Errlog webtest_fn_15: NotFoundError: NotFoundError: DOM Exception 8
Errlog webtest_fn_10: TypeError: Type error
Errlog webtest_fn_15: TypeError: Type error
Errlog webtest_fn_16: TypeError: undefined is not an object (evaluating 'elem.parentNode')
Errlog webtest_fn_18: TypeError: undefined is not an object (evaluating 'document.applets[0].contentEditable="true"')
Errlog webtest_fn_21: TypeError: undefined is not an object (evaluating 'document.anchors[4].appendChild')
138 LayoutTests/http/tests/w3c/dom/interfaces-expected.txt
View file

Large diffs are not rendered by default.

2 LayoutTests/http/tests/w3c/dom/ranges/Range-comparePoint-2-expected.txt
View file
@@ -1,5 +1,5 @@

PASS Range.comparePoint
FAIL Range.comparePoint 1 assert_throws: function "function () { r.comparePoint(null, 0) }" threw object "Error: HierarchyRequestError: DOM Exception 3" ("HierarchyRequestError") expected object "TypeError" ("TypeError")
PASS Range.comparePoint 1
PASS Range.comparePoint 2

2 LayoutTests/http/tests/w3c/dom/ranges/Range-intersectsNode-binding-expected.txt
View file
@@ -1,3 +1,3 @@

FAIL Calling intersectsNode without an argument or with an invalid argument should throw a TypeError. assert_throws: function "function () { r.intersectsNode(); }" threw object "Error: NotFoundError: DOM Exception 8" ("NotFoundError") expected object "TypeError" ("TypeError")
PASS Calling intersectsNode without an argument or with an invalid argument should throw a TypeError.

55 Source/WebCore/ChangeLog
View file
@@ -1,3 +1,58 @@
2015-09-01 Chris Dumez <cdumez@apple.com>

Range API should throw a TypeError for null Node parameters
https://bugs.webkit.org/show_bug.cgi?id=148692

Reviewed by Ryosuke Niwa.

Range API should throw a TypeError for null Node parameters. We currently
throw a NotFoundError.

As per the DOM specification, the Node arguments are not optional nor
nullable [1]:
https://dom.spec.whatwg.org/#range

Therefore, as per the Web IDL specification, we should throw a TypeError
if the Node parameter is null or missing:
https://heycam.github.io/webidl/#es-interface (step 1).

[1] https://heycam.github.io/webidl/#es-nullable-type

No new tests, covered by existing tests which have been
rebaselined.

* dom/Range.cpp:
(WebCore::Range::setStart):
(WebCore::Range::setEnd):
(WebCore::Range::isPointInRange):
(WebCore::Range::comparePoint):
(WebCore::Range::compareNode):
(WebCore::Range::compareBoundaryPoints):
(WebCore::Range::intersectsNode):
(WebCore::Range::insertNode):
(WebCore::Range::setStartAfter):
(WebCore::Range::setEndBefore):
(WebCore::Range::setEndAfter):
(WebCore::Range::selectNode):
(WebCore::Range::selectNodeContents):
(WebCore::Range::surroundContents):
(WebCore::Range::setStartBefore):
Set the Exception code to TypeError instead of NOT_FOUND_ERR if
the Node parameter is null.

* dom/Range.idl:
Stop marking the Node parameters as optional. They are not optional in
the specification and they are not really optional in our implementation.
Previously, if the Node parameter was missing, we would call the
implementation with a null pointer and the implementation would throw a
NotFoundError. Now that they are mandatory, the bindings will directly
throw a TypeError (as per the Web IDL spec) if the Node parameter is
missing. However, if the JavaScript explicitely passes null or undefined,
the implementation will still be called with a null pointer (because
our bindings generator does not distinguish nullable / non-nullable
parameters). For this reason, we still need to handle null pointers on
the implementation side.

2015-09-01 Ryosuke Niwa <rniwa@webkit.org>

Rename ShadowRoot::hostElement to shadowRoot::host to match the latest spec
30 Source/WebCore/dom/Range.cpp
View file
@@ -157,7 +157,7 @@ static inline bool checkForDifferentRootContainer(const RangeBoundaryPoint& star
void Range::setStart(PassRefPtr<Node> refNode, int offset, ExceptionCode& ec)
{
if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

@@ -181,7 +181,7 @@ void Range::setStart(PassRefPtr<Node> refNode, int offset, ExceptionCode& ec)
void Range::setEnd(PassRefPtr<Node> refNode, int offset, ExceptionCode& ec)
{
if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

@@ -225,7 +225,7 @@ void Range::collapse(bool toStart)
bool Range::isPointInRange(Node* refNode, int offset, ExceptionCode& ec)
{
if (!refNode) {
ec = HIERARCHY_REQUEST_ERR;
ec = TypeError;
return false;
}

@@ -249,7 +249,7 @@ short Range::comparePoint(Node* refNode, int offset, ExceptionCode& ec) const
// refNode node and an offset within the node is before, same as, or after the range respectively.

if (!refNode) {
ec = HIERARCHY_REQUEST_ERR;
ec = TypeError;
return 0;
}

@@ -285,7 +285,7 @@ Range::CompareResults Range::compareNode(Node* refNode, ExceptionCode& ec) const
// before and after(surrounds), or inside the range, respectively

if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return NODE_BEFORE;
}

@@ -323,7 +323,7 @@ Range::CompareResults Range::compareNode(Node* refNode, ExceptionCode& ec) const
short Range::compareBoundaryPoints(CompareHow how, const Range* sourceRange, ExceptionCode& ec) const
{
if (!sourceRange) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return 0;
}

@@ -489,7 +489,7 @@ bool Range::intersectsNode(Node* refNode, ExceptionCode& ec) const
// Returns a bool if the node intersects the range.

if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return false;
}

@@ -841,7 +841,7 @@ void Range::insertNode(PassRefPtr<Node> prpNewNode, ExceptionCode& ec)

ec = 0;
if (!newNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

@@ -1082,7 +1082,7 @@ Ref<Range> Range::cloneRange() const
void Range::setStartAfter(Node* refNode, ExceptionCode& ec)
{
if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

@@ -1097,7 +1097,7 @@ void Range::setStartAfter(Node* refNode, ExceptionCode& ec)
void Range::setEndBefore(Node* refNode, ExceptionCode& ec)
{
if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

@@ -1112,7 +1112,7 @@ void Range::setEndBefore(Node* refNode, ExceptionCode& ec)
void Range::setEndAfter(Node* refNode, ExceptionCode& ec)
{
if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

@@ -1127,7 +1127,7 @@ void Range::setEndAfter(Node* refNode, ExceptionCode& ec)
void Range::selectNode(Node* refNode, ExceptionCode& ec)
{
if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

@@ -1185,7 +1185,7 @@ void Range::selectNode(Node* refNode, ExceptionCode& ec)
void Range::selectNodeContents(Node* refNode, ExceptionCode& ec)
{
if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

@@ -1223,7 +1223,7 @@ void Range::surroundContents(PassRefPtr<Node> passNewParent, ExceptionCode& ec)
RefPtr<Node> newParent = passNewParent;

if (!newParent) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

@@ -1309,7 +1309,7 @@ void Range::surroundContents(PassRefPtr<Node> passNewParent, ExceptionCode& ec)
void Range::setStartBefore(Node* refNode, ExceptionCode& ec)
{
if (!refNode) {
ec = NOT_FOUND_ERR;
ec = TypeError;
return;
}

35 Source/WebCore/dom/Range.idl
View file
@@ -32,32 +32,31 @@
readonly attribute boolean collapsed;
readonly attribute Node commonAncestorContainer;

[ObjCLegacyUnnamedParameters, RaisesException] void setStart([Default=Undefined] optional Node refNode,
[ObjCLegacyUnnamedParameters, RaisesException] void setStart(Node refNode,
[Default=Undefined] optional long offset);
[ObjCLegacyUnnamedParameters, RaisesException] void setEnd([Default=Undefined] optional Node refNode,
[ObjCLegacyUnnamedParameters, RaisesException] void setEnd(Node refNode,
[Default=Undefined] optional long offset);
[RaisesException] void setStartBefore([Default=Undefined] optional Node refNode);
[RaisesException] void setStartAfter([Default=Undefined] optional Node refNode);
[RaisesException] void setEndBefore([Default=Undefined] optional Node refNode);
[RaisesException] void setEndAfter([Default=Undefined] optional Node refNode);
[RaisesException] void setStartBefore(Node refNode);
[RaisesException] void setStartAfter(Node refNode);
[RaisesException] void setEndBefore(Node refNode);
[RaisesException] void setEndAfter(Node refNode);
void collapse([Default=Undefined] optional boolean toStart);
[RaisesException] void selectNode([Default=Undefined] optional Node refNode);
[RaisesException] void selectNodeContents([Default=Undefined] optional Node refNode);
[RaisesException] void selectNode(Node refNode);
[RaisesException] void selectNodeContents(Node refNode);

// CompareHow
const unsigned short START_TO_START = 0;
const unsigned short START_TO_END = 1;
const unsigned short END_TO_END = 2;
const unsigned short END_TO_START = 3;

[ObjCLegacyUnnamedParameters, RaisesException, ImplementedAs=compareBoundaryPointsForBindings] short compareBoundaryPoints([Default=Undefined] optional unsigned short how,
[Default=Undefined] optional Range sourceRange);
[ObjCLegacyUnnamedParameters, RaisesException, ImplementedAs=compareBoundaryPointsForBindings] short compareBoundaryPoints([Default=Undefined] optional unsigned short how, Range sourceRange);

[RaisesException] void deleteContents();
[RaisesException] DocumentFragment extractContents();
[RaisesException] DocumentFragment cloneContents();
[RaisesException] void insertNode([Default=Undefined] optional Node newNode);
[RaisesException] void surroundContents([Default=Undefined] optional Node newParent);
[RaisesException] void insertNode(Node newNode);
[RaisesException] void surroundContents(Node newParent);
Range cloneRange();
DOMString toString();

@@ -76,21 +75,17 @@

// WebKit extensions

[RaisesException] boolean intersectsNode([Default=Undefined] optional Node refNode);

[RaisesException] short compareNode([Default=Undefined] optional Node refNode);
[RaisesException] short compareNode(Node refNode);

// CompareResults
const unsigned short NODE_BEFORE = 0;
const unsigned short NODE_AFTER = 1;
const unsigned short NODE_BEFORE_AND_AFTER = 2;
const unsigned short NODE_INSIDE = 3;

[RaisesException] short comparePoint([Default=Undefined] optional Node refNode,
[Default=Undefined] optional long offset);

[RaisesException] boolean isPointInRange([Default=Undefined] optional Node refNode,
[Default=Undefined] optional long offset);
[RaisesException] short comparePoint(Node refNode, [Default=Undefined] optional long offset);
[RaisesException] boolean intersectsNode(Node refNode);
[RaisesException] boolean isPointInRange(Node refNode, [Default=Undefined] optional long offset);

[RaisesException] void expand([Default=Undefined] optional DOMString unit);

0 comments on commit 9432db4

Please sign in to comment.