Skip to content

fix: several issues in NamedNodeMap and Element#482

Merged
karfau merged 9 commits into
masterfrom
46/NamedNodeMap.getNamedItem
Jun 9, 2023
Merged

fix: several issues in NamedNodeMap and Element#482
karfau merged 9 commits into
masterfrom
46/NamedNodeMap.getNamedItem

Conversation

@karfau

@karfau karfau commented Mar 19, 2023

Copy link
Copy Markdown
Member
  • proper order in finding attributes (only affects duplicate attributes, which can only be created manually and anyways produce invalid XML)
  • properly remove item from previous last index (was not iterated over but the reference was kept)
  • set ownerElement to null even when ownerElement.ownerDocument is not set (which is an edge case which doesn't happen when parsing XML)
  • throw DOMException when there is no matching attribute
  • add test coverage
  • add doc comments regarding differences to the specification that still exist:
    • no named property indices support
  • add validation of qualified names in Element API which can throw DOMException as declared in the specs
  • always initialize required properties with null in Attr constructor

BREAKING CHANGES: Mainly not throwing when removing an attribute that doesn't exist and other behavior that was different from the specification.

closes #46
see #477

@karfau karfau added the breaking change Some thing that requires a version bump due to breaking changes label Mar 19, 2023
@karfau karfau added this to the 0.9.0 milestone Mar 19, 2023
@karfau karfau added bug Something isn't working spec:DOM Living Standard https://dom.spec.whatwg.org/ spec:DOM Level 3 https://www.w3.org/TR/DOM-Level-3-Core/ labels Mar 19, 2023
@karfau

karfau commented Mar 19, 2023

Copy link
Copy Markdown
Member Author

According to the spec the removeAttribute* methods on Element should not throw (the same way) when an attribute is not found, but I think they would with the current implementation.
Since those methods are also not covered with tests, I will add tests for those before merging.

@karfau karfau force-pushed the 46/NamedNodeMap.getNamedItem branch 2 times, most recently from b4fc55b to 2994bc2 Compare April 2, 2023 11:03
karfau added 2 commits April 2, 2023 16:38
- properly remove item from previous last index
- set `ownerElement` to `null` even when `ownerElement.ownerDocument` is not set
- throw DOMException when there is no matching attribute
- add test coverage
- drop unreachable code
@karfau karfau force-pushed the 46/NamedNodeMap.getNamedItem branch from 2994bc2 to 9658a0b Compare April 2, 2023 14:38
karfau added 4 commits June 3, 2023 21:42
and apply it everywhere.
- Initialize `Attr` with required `null` values.
- make `NamedNodeMap.setNamedItemNS` an alias for `NamedNodeMap.setNamedItem`

some tests still need cleanup
Comment thread test/dom/attr.test.js Fixed
Comment thread test/dom/named-node-map.test.js Fixed
@karfau karfau changed the title fix: several issues in NamedNodeMap fix: several issues in NamedNodeMap and Element Jun 9, 2023
@karfau karfau merged commit 9e65be6 into master Jun 9, 2023
@karfau karfau deleted the 46/NamedNodeMap.getNamedItem branch June 9, 2023 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Some thing that requires a version bump due to breaking changes bug Something isn't working spec:DOM Level 3 https://www.w3.org/TR/DOM-Level-3-Core/ spec:DOM Living Standard https://dom.spec.whatwg.org/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getAttribute() doesn't return null for not existing attributes

2 participants