Skip to content

Copy the DOM XPath interfaces from the WHATWG wiki - #763

Merged
annevk merged 5 commits into
masterfrom
foolip/xpath-idl
Aug 30, 2019
Merged

annevk merged 5 commits into
masterfrom
foolip/xpath-idl

Conversation

@foolip

@foolip foolip commented May 29, 2019 •

Copy link
Copy Markdown
Member

@foolip

foolip commented May 29, 2019

Copy link
Copy Markdown
Member Author

Tests using this IDL already exist:
https://wpt.fyi/results/domxpath/interfaces.tentative.html?run_id=258200004&run_id=264060011&run_id=250500005&run_id=266120012

They show that all of this is already implemented, the failures in Edge and Safari are because of minor differences in length properties, what exception is thrown, and so on.

Comment thread dom.bs Outdated
Comment thread dom.bs Outdated
Comment thread dom.bs Outdated
Comment thread dom.bs Outdated
Comment thread dom.bs Outdated
Comment thread dom.bs Outdated
Comment thread dom.bs Outdated
[[DOM-Level-3-XPath]] [[XPath]] These APIs are widely implemented, but
<cite>DOM Level 3 XPath</cite> is no longer maintained. The DOM Standard does not attempt to
define any of the behavior, but the interface definitions are maintained here so that they can
be updated when Web IDL changes. [[WEBIDL]]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's more changes than just that, some of them simplify the API by making arguments optional.

Let's also add something, possibly just normative, saying:

  • XPathException is gone (just use DOMException)
  • The query is matched against the DOM, and therefore contrary to the XPath 1.0 data model the root element has a parent (the Document) and text nodes can be adjacent to one another. [wilful violation]

@annevk annevk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you want to improve this further or have me do it, but there's two changes I'd like to see here before merging this:

  1. The first paragraph should be reworded and flagged as an issue (using class=issue), ideally linking to an open issue in the DOM Standard that would allow for someone to take on the work of defining the API in more detail. Sam's points can be included in that issue description. I don't think we should have normative wording outside of the issue.
  2. The note should probably be reworded a bit to be slightly more formal.

Also, if anyone has issues with merging this, please speak up, as I don't have a good reason to not do this at this point.

@foolip

foolip commented Jul 19, 2019

Copy link
Copy Markdown
Member Author

I'll polish this up next week.

@foolip

foolip commented Jul 25, 2019

Copy link
Copy Markdown
Member Author

I didn't get to this this week and I'm OOO tomorrow. Snoozing it until next week but won't be offended if someone takes it over.

@foolip

foolip commented Aug 28, 2019

Copy link
Copy Markdown
Member Author

@annevk I pushed the changes. However, class=issue doesn't seem to be a thing, so I used class=warning. Don't love the way it looks, is there a better class?

@zcorpan

zcorpan commented Aug 28, 2019

Copy link
Copy Markdown
Member

@foolip class=XXX

@foolip

foolip commented Aug 28, 2019

Copy link
Copy Markdown
Member Author

@zcorpan thanks, trying that instead

@annevk

annevk commented Aug 28, 2019

Copy link
Copy Markdown
Member

This is blocked by w3ctag/promises-guide#27 for now. I adjusted the wording a bit to make it clear that defining these is still a goal, albeit perhaps with less priority than some would like.

@foolip

foolip commented Aug 30, 2019

Copy link
Copy Markdown
Member Author

w3ctag/promises-guide#27 is resolved, Travis now happy.

@annevk
annevk merged commit dea5d92 into master Aug 30, 2019
@annevk
annevk deleted the foolip/xpath-idl branch August 30, 2019 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants