The default bug view has changed. See this FAQ.

CSSStyleSheet.insertRule should throw when there are more than one rule

RESOLVED FIXED in mozilla21

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: kennyluck, Assigned: dbaron)

Tracking

({dev-doc-complete})

unspecified
mozilla21
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
See URL for the original spec error report. I think the spec is OK clear in this case, though.

Returning a NS_ERROR_DOM_SYNTAX_ERR if there's any more token after the conditional in [1] seems to work. It might be helpful to read [2] too, to see if some other codes can be cleaned up.

The hard part might be finding a file in /layout/style/test/ to put this test case in...

[1] http://hg.mozilla.org/mozilla-central/file/b1a0fb2bdbf7/layout/style/nsCSSParser.cpp#l1015
[2] http://hg.mozilla.org/mozilla-central/file/b1a0fb2bdbf7/layout/style/nsCSSStyleSheet.cpp#l1765

Comment 1

5 years ago
can I be assigned this bug???
(Reporter)

Comment 2

5 years ago
(In reply to rajesh kumar from comment #1)
> can I be assigned this bug???

Yes, and I just did that. I am sending you a mail for further instructions.
Assignee: nobody → mailto.rajesh05
Status: NEW → ASSIGNED
(Assignee)

Comment 3

4 years ago
I just wrote a patch for this, after clarifying the spec, before finding this bug report.


The spec issue, from the css3-conditional disposition of comments, is:

Issue 5:
Issue in spec about insertRule, followup to Issue 7
also raised as first issue in http://lists.w3.org/Archives/Public/www-style/2013Feb/0228.html
Raised by: David Baron, Florian Rivoal
Proposed resolution: http://lists.w3.org/Archives/Public/www-style/2013Feb/0229.html
Florian agrees: http://lists.w3.org/Archives/Public/www-style/2013Feb/0228.html
Edited: http://lists.w3.org/Archives/Public/www-style/2013Feb/0256.html
(Commenter agrees, of course.)
Assignee: mailto.rajesh05 → dbaron
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [good first bug][mentor=kennyluck][lang=c++]
(Assignee)

Comment 4

4 years ago
Created attachment 712083 [details] [diff] [review]
Make CSS insertRule methods throw SYNTAX_ERR when given an empty rule or more than one rule.

This implements the proposed spec clarification in
http://lists.w3.org/Archives/Public/www-style/2013Feb/0229.html which
makes us compatible with WebKit on the insertRule tests in this patch.

I confirmed that the test reports 7 failures without the patch, but
passes with the patch.  (I'm a little disturbed by the way our
testharness.js integration elides runs of successive passes.)
Attachment #712083 - Flags: review?(bzbarsky)
Comment on attachment 712083 [details] [diff] [review]
Make CSS insertRule methods throw SYNTAX_ERR when given an empty rule or more than one rule.

Reporting PERuleTrailing when !*aResult is a bit odd.  Shouldn't we only do that when GetToken() returns true?

Given that we now check for whitespace-only things in the CSS parser and throw,
do we still need the explicit IsEmpty() checks in nsCSSStyleSheet::InsertRuleInternal and nsCSSStyleSheet::InsertRuleIntoGroup?

r=me with those addresed.
Attachment #712083 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 6

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bb11a09bb20
https://hg.mozilla.org/mozilla-central/rev/5bb11a09bb20
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
I've added this bug to the compatibility doc. Please correct the info if I'm wrong.
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_21
I also added a note in:
https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleSheet/insertRule
and
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/21
Keywords: dev-doc-complete
You need to log in before you can comment on or make changes to this bug.