Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw InvalidNodeTypeError on DocumentType nodes #118

Closed
pshaughn opened this issue Feb 1, 2020 · 5 comments · Fixed by #342
Closed

Throw InvalidNodeTypeError on DocumentType nodes #118

pshaughn opened this issue Feb 1, 2020 · 5 comments · Fixed by #342

Comments

@pshaughn
Copy link

pshaughn commented Feb 1, 2020

Various methods that take a node and offset are specified as throwing IndexSizeError if the offset exceeds the node's length.
By WPT tests and by analogy with Range methods, these also need to throw InvalidNodeTypeError if the node is a DocumentType, and InvalidNodeType is the condition to check first for cases where both apply.

@pshaughn pshaughn changed the title Throw InvalidNodeType on DocumentType nodes Throw InvalidNodeTypeError on DocumentType nodes Feb 1, 2020
@rniwa
Copy link
Contributor

rniwa commented Feb 4, 2020

Do all major browsers currently throw InvalidNodeTypeError?

@pshaughn
Copy link
Author

pshaughn commented Feb 6, 2020

Chrome, Firefox, and Edge do; Safari might not. https://wpt.fyi/results/selection/selectAllChildren.html?label=experimental&label=master&aligned has many cases involving a doctype.

@darinadler
Copy link

The specification already requires throwing InvalidNodeTypeError. Not directly, but indirectly. The specification has steps that say things like “set the start of newRange” and those steps will throw InvalidNodeTypeError because that’s part of the set the start or end algorithm from the DOM specification.

@darinadler
Copy link

Regarding the ordering of the checks, see also my comments in #124.

@darinadler
Copy link

darinadler commented Sep 20, 2020

Just looked at the WPT test and unfortunately it makes does the DOCUMENT_TYPE_NODE check before making the document.contains check, so it expects behavior different from what the specification currently requires.

awesomekling added a commit to awesomekling/selection-api that referenced this issue Nov 16, 2024
This matches how major engines behave and is covered by multiple
tests in WPT already.

Closes w3c#118.
rniwa pushed a commit that referenced this issue Nov 20, 2024
This matches how major engines behave and is covered by multiple
tests in WPT already.

Closes #118.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants