Last Comment Bug 605982 - mozMatchesSelector should raise a SYNTAX_ERR exception on invalid selectors
: mozMatchesSelector should raise a SYNTAX_ERR exception on invalid selectors
[good first bug]
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla2.0b7
Assigned To: Mounir Lamouri (:mounir)
: Andrew Overholt [:overholt]
javascript: try { document.documentEl...
Depends on:
Blocks: 518003
  Show dependency treegraph
Reported: 2010-10-20 14:54 PDT by Matt Cosentino
Modified: 2011-01-03 14:45 PST (History)
6 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (5.04 KB, patch)
2010-10-20 16:02 PDT, Mounir Lamouri (:mounir)
bzbarsky: review-
Details | Diff | Splinter Review
Patch v2 (5.19 KB, patch)
2010-10-21 03:53 PDT, Mounir Lamouri (:mounir)
bzbarsky: review+
bzbarsky: approval2.0+
Details | Diff | Splinter Review

Description Matt Cosentino 2010-10-20 14:54:32 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv: Gecko/20101012 Firefox/3.6.11
Build Identifier: 

Currently mozMatchesSelector returns false when called with an invalid selector, this is the wrong behavior.  Here is what the W3C draft says:

"If the given group of selectors is invalid ([SELECT], section 13), the implementation must raise a SYNTAX_ERR exception ([DOM-LEVEL-3-CORE], section 1.4)."

The webkit and ie implementations of this method correctly raise the exception.

Reproducible: Always

Steps to Reproduce:
1. Call mozMatchesSelector with an invalid selector.
Actual Results:  
returns false

Expected Results:  
raises SYNTAX_ERR exception
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2010-10-20 15:41:29 PDT
Compatibility concern, content/base/src/nsGenericElement.cpp looks easy to fix even for someone not really familiar with the code, should address for this release, I think.
Comment 2 Mounir Lamouri (:mounir) 2010-10-20 16:02:02 PDT
Created attachment 484864 [details] [diff] [review]
Patch v1

I'm wondering if we shouldn't add the tests to content/base/test/file_bug416317.xhtml?
Comment 3 Mounir Lamouri (:mounir) 2010-10-20 16:02:36 PDT
By the way, thank you for your report, Matt :)
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2010-10-20 17:14:52 PDT
Comment on attachment 484864 [details] [diff] [review]
Patch v1

I'm fine either way on the test, but you should be propagating out the actual error from ParseSelectorList instead of assuming it's always SYNTAX_ERR.
Comment 5 Mounir Lamouri (:mounir) 2010-10-21 03:53:45 PDT
Created attachment 485006 [details] [diff] [review]
Patch v2
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2010-10-21 06:38:45 PDT
Comment on attachment 485006 [details] [diff] [review]
Patch v2

Comment 7 Mounir Lamouri (:mounir) 2010-10-21 08:26:26 PDT

Thank you for your report Matt :)
Comment 8 Eric Shepherd [:sheppy] 2010-11-03 12:32:43 PDT
Updated documentation:

Noted on Firefox 4 for developers.

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