Last Comment Bug 604172 - at-import-009 in CSS 2.1 test suite fails
: at-import-009 in CSS 2.1 test suite fails
Status: RESOLVED FIXED
: css2
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla5
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
:
Mentors:
http://test.csswg.org/suites/css2.1/2...
Depends on:
Blocks: css2.1-tests
  Show dependency treegraph
 
Reported: 2010-10-13 14:20 PDT by David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
Modified: 2011-03-23 13:46 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 1: missing UngetToken (730 bytes, patch)
2010-10-19 17:05 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review
patch 2: use more stop characters in GatherMedia (7.16 KB, patch)
2010-10-19 17:08 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Splinter Review

Description David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-10-13 14:20:15 PDT
http://test.csswg.org/suites/css2.1/20100917/xhtml1/at-import-009.xht fails; there are actually two underlying bugs:
 (1) we're missing an UngetToken in error handling of parsing @charset rules
 (2) GatherMedia needs to stop at a block or semicolon in invalid cases

I have a patch for both in my patch queue, but I seem to recall that I wasn't quite happy with the patch for the second:

http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/6038c95fb322/at-charset-unget-token
http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/6038c95fb322/gather-media-stop-in-at-rule
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-10-19 17:05:58 PDT
Created attachment 484548 [details] [diff] [review]
patch 1: missing UngetToken
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-10-19 17:08:08 PDT
Created attachment 484550 [details] [diff] [review]
patch 2: use more stop characters in GatherMedia

In theory, stop characters are a bad approach for everything except skipping over garbage.  I thought about removing other uses, but that makes error handling hard.  However, including extra stop chars isn't harmful in the other cases (other than skipping over garbage), since they're not valid syntax.
Comment 3 Boris Zbarsky [:bz] 2010-10-19 20:22:55 PDT
Comment on attachment 484548 [details] [diff] [review]
patch 1: missing UngetToken

r=me
Comment 4 Boris Zbarsky [:bz] 2010-10-19 20:24:50 PDT
Comment on attachment 484550 [details] [diff] [review]
patch 2: use more stop characters in GatherMedia

r=me
Comment 5 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-03-03 19:05:02 PST
https://hg.mozilla.org/projects/birch/rev/35b1ff9a626e
https://hg.mozilla.org/projects/birch/rev/60cef935a89c
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-03-23 13:46:05 PDT
https://hg.mozilla.org/mozilla-central/rev/35b1ff9a626e
https://hg.mozilla.org/mozilla-central/rev/60cef935a89c

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