The default bug view has changed. See this FAQ.

mozMatchesSelector should raise a SYNTAX_ERR exception on invalid selectors

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
DOM
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: Matt Cosentino, Assigned: mounir)

Tracking

({dev-doc-complete})

Trunk
mozilla2.0b7
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 -)

Details

(Whiteboard: [good first bug], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.11) 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
(Reporter)

Updated

7 years ago
OS: Windows 7 → All
Hardware: x86 → All
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.
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
Whiteboard: [good first bug]
(Assignee)

Updated

7 years ago
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Version: unspecified → Trunk
(Assignee)

Comment 2

7 years ago
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?
Attachment #484864 - Flags: review?(bzbarsky)
Attachment #484864 - Flags: approval2.0?
(Assignee)

Comment 3

7 years ago
By the way, thank you for your report, Matt :)
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.
Attachment #484864 - Flags: review?(bzbarsky) → review-
(Assignee)

Updated

7 years ago
Blocks: 518003
(Assignee)

Comment 5

7 years ago
Created attachment 485006 [details] [diff] [review]
Patch v2
Attachment #484864 - Attachment is obsolete: true
Attachment #485006 - Flags: review?(bzbarsky)
Attachment #484864 - Flags: approval2.0?
(Assignee)

Updated

7 years ago
Attachment #485006 - Flags: approval2.0?
Comment on attachment 485006 [details] [diff] [review]
Patch v2

r=me
Attachment #485006 - Flags: review?(bzbarsky)
Attachment #485006 - Flags: review+
Attachment #485006 - Flags: approval2.0?
Attachment #485006 - Flags: approval2.0+
(Assignee)

Comment 7

7 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/25507af6a959

Thank you for your report Matt :)
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Keywords: dev-doc-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8

Updated

7 years ago
Target Milestone: mozilla2.0b8 → mozilla2.0b7
Updated documentation:

https://developer.mozilla.org/en/DOM/Node.mozMatchesSelector

Noted on Firefox 4 for developers.
Keywords: dev-doc-needed → dev-doc-complete

Updated

6 years ago
blocking2.0: ? → -
You need to log in before you can comment on or make changes to this bug.