Closed
Bug 887741
Opened 12 years ago
Closed 12 years ago
Allow CSS at-rules in declaration lists
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: SimonSapin, Assigned: SimonSapin)
References
()
Details
(Keywords: dev-doc-needed, Whiteboard: [DocArea=CSS])
Attachments
(1 file, 1 obsolete file)
|
4.36 KB,
patch
|
Details | Diff | Splinter Review |
CSS 2.1 defines @page rules as containing at-rules and declarations, even though no at-rule supported in that context is defined yet. (Such rules are defined in CSS Paged Media Level 3.)
http://www.w3.org/TR/CSS21/page.html#page-box
Anything unknown/invalid is already ignored by the parser, but being aware of at-rules makes a difference is where the parser recovers and starts parsing the next declaration:
@page {
/* Level 3 stuff, not supported in Mozilla yet: */
@bottom-center { content: counter(page) }
margin: 3cm;
}
Per CSS 2.1, the @bottom-center rule ends with its {} block, and is followed by a valid declaration. Our current behavior is to consider the at-rule an invalid declaration that goes until the next semi-colon, including the 'margin' declaration, and ignore all of it.
Test case, print preview should show a big margin around "Test":
data:text/html,<!doctype html><style>@page{margin:0;@top-left{}margin:5cm}</style>Test
Sanity check: Page margins are supported at all.
data:text/html,<!doctype html><style>@page{margin:0;margin:5cm}</style>Test
Additionally, the CSS WG recently decided to extend this behavior to all declaration lists, not just @page. This is specified in Syntax Level 3. As for @page in CSS 2.1, this error recovery allows future extensions. It should be done early even if we don’t define such extensions yet.
http://www.w3.org/blog/CSS/2013/06/10/resolutions-94/
http://dev.w3.org/csswg/css-syntax/#parse-a-list-of-declarations
Test case for style rules:
data:text/html,<!doctype html><style>body{background:red;@foo{}background:green}</style>Test
Test case for style attributes:
data:text/html,<!doctype html><body style="background:red;@foo{}background:green">Test
This should apply similarly to at-rules that use (descriptor) declaration syntax.
|
Assignee | |
Comment 1•12 years ago
|
||
The test I added passes with this code change, but I’m waiting for push access to Try to see if this broke anything else.
Attachment #777808 -
Flags: review?(dbaron)
|
|
Assignee | |
Comment 2•12 years ago
|
||
Try results: https://tbpl.mozilla.org/?tree=Try&rev=2fca9551cc28
The failures seem unrelated.
Comment on attachment 777808 [details] [diff] [review]
First patch: implementation + one mochitest
>+ if(eCSSToken_AtKeyword == tk->mType) {
space between if and (.
>+ SkipAtRule(checkForBraces);
>+ return true; // Not a declaration, but not not skip until ';'
s/not not/don't/
(or "do not")
It probably would have been better to write this as a testharness.js test so it could be contributed to the css3-syntax test suite, but probably not worth redoing now
r=dbaron
Attachment #777808 -
Flags: review?(dbaron) → review+
|
|
Assignee | |
Comment 4•12 years ago
|
||
Is testharness.js supported in Mozilla code? https://developer.mozilla.org/en-US/docs/Mozilla/QA/Automated_testing doesn’t mention it. Anyway, this should be easy enough to port.
|
|
Assignee | |
Comment 5•12 years ago
|
||
Attachment #777808 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•12 years ago
|
Keywords: checkin-needed
|
||
Comment 6•12 years ago
|
||
Keywords: checkin-needed
|
|
||
Comment 7•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
|
||
Updated•12 years ago
|
Keywords: dev-doc-needed
|
||
Updated•11 years ago
|
Whiteboard: [DocArea=CSS]
You need to log in
before you can comment on or make changes to this bug.
Description
•