Closed Bug 886308 Opened 10 years ago Closed 9 years ago

Implement Element.matches()

Categories

(Core :: DOM: CSS Object Model, defect)

Product:
Core
Component:
DOM: CSS Object Model
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla34
Tracking Flags:
Tracking Status
relnote-firefox --- 34+

People

(Reporter: bruant.d, Assigned: bzbarsky)

References

(Blocks 2 open bugs,
URL
)

Details

(Keywords: dev-doc-complete, feature)

Attachments

(1 file)

Implement Element.matches
4.76 KB, patch
smaug
: review+
Details | Diff | Splinter Review 
The Selector spec seems to have settled on "matches"
URL: http://dev.w3.org/2006/webapi/selecto...
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)?
Keywords: site-compat
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.
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()
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.
URL: http://dev.w3.org/2006/webapi/selecto... → http://dom.spec.whatwg.org/#dom-eleme...
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)
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)
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>
Cameron, how close is our current implementation to the spec?
Flags: needinfo?(cam)
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)
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?
(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.
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.
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?
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.
I really don't have a strong opinion on the exception situation; I haven't put enough thought into it.
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

        Attachment #8466350 -
        Flags: review?(bugs)

    
  
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED

    
  
Whiteboard: [need review]

        Attachment #8466350 -
        Flags: review?(bugs) → review+

    
    

    
  
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6a94e4dc22b
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla34

    
    

    
  
Release Note Request (optional, but appreciated)
[Why is this notable]:
[Suggested wording]:
[Links (documentation, blog post, etc)]:
Keywords: feature, 
          
            relnote

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/c6a94e4dc22b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED

    
    

    
  
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

    
    

    
  
I'll add this to the site compat doc soon.
Keywords: dev-doc-needed

    
    

    
  
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.

    
    

    
  
Thanks a lot!

    
  
Flags: needinfo?(paul)

    
    

    
  
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)

    
    

    
  
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.

    
    

    
  
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)

    
    

    
  
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

    
    

    
  
Both Fx 34 for devs and Site compat for 34 has a mention now -> ddc
Keywords: dev-doc-needed → dev-doc-complete
Flags: needinfo?(cam)

          You need to log in
          before you can comment on or make changes to this bug.