Closed
Bug 886308
Opened 11 years ago
Closed 10 years ago
Implement Element.matches()
Categories
(Core :: DOM: CSS Object Model, defect)
Core
DOM: CSS Object Model
Tracking
()
RESOLVED
FIXED
mozilla34
| Tracking | Status | |
|---|---|---|
| relnote-firefox | --- | 34+ |
People
(Reporter: bruant.d, Assigned: bzbarsky)
References
(Blocks 2 open bugs, )
Details
(Keywords: dev-doc-complete, feature)
Attachments
(1 file)
|
4.76 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
The Selector spec seems to have settled on "matches"
|
Reporter | |
Updated•11 years ago
|
Component: Layout → DOM: Core & HTML
Keywords: dev-doc-needed
Component: DOM: Core & HTML → DOM: CSS Object Model
Do people have a good idea how stable the WG thinks this is, i.e., whether it's stable enough that it's worth renaming (and unprefixing)?
|
||
Updated•11 years ago
|
Keywords: site-compat
|
||
Comment 2•11 years ago
|
||
Note that matches() support relative selectors (now defined by Selectors 4) and has a second argument, etc. It's quite a bit different from the original.
|
Assignee | |
Comment 3•11 years ago
|
||
Indeed. This bug as filed makes no sense; we should add a matches() alongside then think about how to go about removing mozMatchesSelector, which is in pretty widespread use. :(
Keywords: site-compat
Summary: Rename mozMatchesSelector to matches → Implement Element.matches()
|
|
||
Comment 4•11 years ago
|
||
I think what I said in comment 2 is no longer true, although I don't quite know how mozMatchesSelector() works. In particular we decided that relative selectors and the second argument were not needed for matches() after all.
|
|
Assignee | |
Comment 5•11 years ago
|
||
Is there a test suite? It's not clear to me what the :scope behavior of our code is at this point and whether it matches the spec.
The DOM spec also needs to say whether this uses scope-contained selectors or scope-filtered ones.
Flags: needinfo?(annevk)
|
|
||
Comment 6•11 years ago
|
||
I'm not aware of a test suite. http://lists.w3.org/Archives/Public/www-style/2013Sep/0243.html is about coordination with Selectors. I think everything in DOM is scope-filtered, I'm hoping that'll be the default matching algorithm.
Flags: needinfo?(annevk)
|
||
Comment 7•11 years ago
|
||
Any plain to use matches() instead of mozMatchesSelector()? Last stable Chrome 34 start using correct name. SELECTORS and DOM spec at this moment are more accurate.
Some test:
<!DOCTYPE html>
<html>
<head>
<script>
function matchElement(selectors){
var context = document.getElementById("test");
var info = document.getElementById("info");
var result = "";
var sel = "('" + selectors + "') :";
if (context.matches){
result += "matches: " + context.matches(selectors);
}
else if (context.mozMatchesSelector){
result += "mozMatchesSelector" + sel + context.mozMatchesSelector(selectors);
}
else if (context.webkitMatchesSelector){
result += "webkitMatchesSelector" + sel + context.webkitMatchesSelector(selectors);
}
else if (context.msMatchesSelector){
result += "msMatchesSelector" + sel + context.msMatchesSelector(selectors);
}
info.innerHTML = result;
}
</script>
</head>
<body>
<p id="test" class="class1 class2">Paragraph being the object context (id="test1", class="class1 class2").</p>
<p>Click the button to test paragraph based on passed selector.</p>
<input type="button" value="matches('p')" onclick="matchElement('p')">
<input type="button" value="matches('#test')" onclick="matchElement('#test')">
<input type="button" value="matches('.class1')" onclick="matchElement('.class1')">
<input type="button" value="matches('.class1.class2')" onclick="matchElement('.class1.class2')">
<input type="button" value="matches('#test2, .class1')" onclick="matchElement('#test2, .class1')">
<input type="button" value="matches(':scope')" onclick="matchElement(':scope')">
<input type="button" value="matches('body :scope')" onclick="matchElement('body :scope')">
<input type="button" value="matches('html body p')" onclick="matchElement('html body p')">
<input type="button" value="matches('body p')" onclick="matchElement('body p')">
<input type="button" value="matches('div') << false" onclick="matchElement('div')">
<p style="color: blue;">More information:</p>
<p id="info"></p>
</body>
</html>
|
|
Assignee | |
Comment 8•11 years ago
|
||
Cameron, how close is our current implementation to the spec?
Flags: needinfo?(cam)
|
||
Comment 9•11 years ago
|
||
I think the implementation matches the spec except for error reporting; we'll throw some XPCOM exception rather than a TypeError when selector parsing fails, which is what the spec requires.
Flags: needinfo?(cam)
|
|
Assignee | |
Comment 10•11 years ago
|
||
Hmm. So right now we share the same selector parsing code as querySelector, and throw a SyntaxError. That's what http://dev.w3.org/2006/webapi/selectors-api2/#parse-a-relative-selector says to do.
What we don't implement is all the jazz about parsing things that are not actually selectors (e.g. that start with a combinator), right?
|
|
||
Comment 11•11 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #10)
> Hmm. So right now we share the same selector parsing code as querySelector,
> and throw a SyntaxError.
Ah, yes, a SyntaxError DOMException. (Sorry was just guessing about the XPCOM stuff tbh!)
> That's what
> http://dev.w3.org/2006/webapi/selectors-api2/#parse-a-relative-selector says
> to do.
http://dom.spec.whatwg.org/#match-a-selectors-string (which I gather is the current spec to work from) says TypeError.
> What we don't implement is all the jazz about parsing things that are not
> actually selectors (e.g. that start with a combinator), right?
Right. But that's not required by http://dom.spec.whatwg.org/#dom-element-matches and the "match a selectors string" links to http://dev.w3.org/csswg/selectors/#parse-a-selector, which parses absolute selectors.
|
|
Assignee | |
Comment 12•11 years ago
|
||
Aha, I see. So it looks like the exception type got changed for querySelector too?
Should be pretty simple to replace this bit in nsINode::ParseSelectorList:
2368 aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
with the relevant ThrowTypeError call.
|
|
||
Comment 13•11 years ago
|
||
Opera has requested we change back to "SyntaxError": https://www.w3.org/Bugs/Public/show_bug.cgi?id=24028
I was hoping to slowly upgrade us to use mostly JavaScript exception types, but it seems that goal is not universally shared and we do not really have a complete story for exceptions anyway.
What do you guys think?
|
|
||
Comment 14•11 years ago
|
||
Yeah, I feel not everyone is on the same page for what to do with exceptions in the future, especially for the fundamental question of "should we have fine grained exception types/names", regardless of whether that's through DOMExceptions or separate things inheriting from Error or even Error but distinguishing via ".name". I think once people have a shared understanding of whether fine grained is the way to go or not, then we can decide what that means in terms of actual objects.
|
|
Assignee | |
Comment 15•11 years ago
|
||
I really don't have a strong opinion on the exception situation; I haven't put enough thought into it.
|
|
||
Comment 16•11 years ago
|
||
Finally DOM for matches() method changed the requirements from TypeError to SyntaxError:
http://dom.spec.whatwg.org/#match-a-selectors-string
https://www.w3.org/Bugs/Public/show_bug.cgi?id=24028
But is still open issue for other methods derived from Selector API, like querySelector() or querySelectorAll():
https://www.w3.org/Bugs/Public/show_bug.cgi?id=25958
|
|
Assignee | |
Comment 17•10 years ago
|
||
Attachment #8466350 -
Flags: review?(bugs)
|
|
Assignee | |
Updated•10 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
|
|
Assignee | |
Updated•10 years ago
|
Whiteboard: [need review]
|
||
Updated•10 years ago
|
Attachment #8466350 -
Flags: review?(bugs) → review+
|
|
Assignee | |
Comment 18•10 years ago
|
||
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla34
|
||
Comment 19•10 years ago
|
||
Release Note Request (optional, but appreciated)
[Why is this notable]:
[Suggested wording]:
[Links (documentation, blog post, etc)]:
|
||
Comment 20•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
|
|
||
Comment 21•10 years ago
|
||
This is new … sorry!
(In reply to Florian Bender from comment #19)
> Release Note Request (optional, but appreciated)
[Why is this notable]: New web tech (unprefixed standardized version of prior implementation)
[Suggested wording]: Implemented `matches()` DOM API (formerly `mozMatchesSelector()`).
[Links (documentation, blog post, etc)]: https://developer.mozilla.org/en-US/docs/Web/API/Element.matches
|
||
Comment 22•10 years ago
|
||
Already documented: https://developer.mozilla.org/en-US/docs/Web/API/Element.matches
Keywords: dev-doc-needed
|
|
||
Comment 23•10 years ago
|
||
May you please add this to https://developer.mozilla.org/en-US/Firefox/Releases/34 and maybe https://developer.mozilla.org/en-US/Firefox/Releases/34/Site_Compatibility ?
Flags: needinfo?(paul)
|
||
Comment 25•10 years ago
|
||
I've added it to https://developer.mozilla.org/en-US/Firefox/Releases/34
I'll leave to Kohei to turn to dev-doc-complete once he has updated his nice site compat page.
|
|
||
Comment 26•10 years ago
|
||
Thanks a lot!
|
|
||
Updated•10 years ago
|
Flags: needinfo?(paul)
|
||
Updated•10 years ago
|
Flags: qe-verify-
|
|
Assignee | |
Comment 27•10 years ago
|
||
Hrm. Did the spec change again? Our implementation will return true for element.matches(":scope") but it's not clear to me that this is what the spec is calling for...
Flags: needinfo?(cam)
|
|
||
Comment 28•10 years ago
|
||
The only change I made to the specification was to use the new primitive you requested as far as I can recall. See https://github.com/whatwg/dom/commit/66c21705d9d3600574f4ca2dd588d040be463d11 for that change.
|
|
Assignee | |
Comment 29•10 years ago
|
||
That changed the behavior, yes. The old setup ended up using the element itself as the :scope, while the new one does not, no?
Flags: needinfo?(annevk)
|
|
||
Comment 30•10 years ago
|
||
I will fix that: http://lists.w3.org/Archives/Public/www-dom/2014JulSep/0102.html
Flags: needinfo?(annevk)
|
||
Comment 31•10 years ago
|
||
Added to the release notes with "matches() DOM API implemented (formerly mozMatchesSelector())".
By the way, please use the tracking flags and not the "relnotes" keyword.
relnote-firefox:
--- → 34+
Keywords: relnote
|
|
||
Comment 32•10 years ago
|
||
Both Fx 34 for devs and Site compat for 34 has a mention now -> ddc
Keywords: dev-doc-needed → dev-doc-complete
|
|
||
Updated•7 years ago
|
Flags: needinfo?(cam)
You need to log in
before you can comment on or make changes to this bug.
Description
•