Closed
Bug 461266
Opened 16 years ago
Closed 16 years ago
error in final media query in list causes media query list to absorb everything until the next comma
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
mozilla1.9.1b2
People
(Reporter: smichaud, Assigned: dbaron)
References
()
Details
(Keywords: regression, testcase)
Attachments
(2 files, 1 obsolete file)
481 bytes,
text/html
|
Details | |
6.13 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
A Flash plugin loaded by this bug's URL (http://video.yahoo.com/) displays in the wrong location (too high on the page), and overlays other elements that it shouldn't overlay (including a couple of tickers). This bug is spun off from bug 459530 comment #45 and following, and was originally reported by Tony Chung. He also found the following regression range for the bug, which is the same on OS X, Windows and Linux: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-10-11+02%3A00%3A00&enddate=2008-10-12+03%3A00%3A00 Nothing stands out (to me) in that list as the cause/trigger for this bug. But I wonder, dbaron, if any of the changes you landed on Sat Oct 11 17:50:38 2008 -0700 might be responsible -- particularly changeset 23eebebb8b48 (bug 322475, "Construct all our image loaders when we create frames ..."). If my hunch is correct, this is a layout bug (and not a plugins bug).
Flags: wanted1.9.1?
Flags: blocking1.9.1?
Comment 1•16 years ago
|
||
i would doubt its a plugins bug, since it worked on 10/11 build but not 10/12. this repros with both flash 9 and 10.
Component: Plug-ins → Layout
QA Contact: plugins → layout
Comment 2•16 years ago
|
||
Dbaron, can you take a look at this bug? This should be a blocking 1.9.1 issue on video.yahoo.com, and needs to be fixed. A regression range is on comment 0. Thanks, Tony
Comment 3•16 years ago
|
||
This bug has nothing to do with Flash. Afaics this is related to float. I'm preparing a minimized testcase.
Keywords: regression
Comment 4•16 years ago
|
||
I was able to reduce the data to a minimized testcase. It shows that a broken CSS declaration stops the parser from processing subsequent selectors. On the given website the line "@media all and(min-width:0)" gives a really huge warning which lists all subsequent selectors: Warning: Expected ',' in media list but found 'and'. Expected identifier in media list but found '#email_from'. Expected identifier in media list but found '#email_msg'. Expected identifier in media list but found '#player_embed'. Expected identifier in media list but found '#addToPl'. [...] Expected identifier in media list but found '#hottopics'. Expected identifier in media list but found '#videoblog'. Expected identifier in media list but found '#categories'. Expected identifier in media list but found '#videoblog'. Expected identifier for pseudo-class or pseudo-element but found '93%'. Ruleset ignored due to bad selector. Source File: file:///Users/henrik/Desktop/Yahoo%21%20Video%20-%20It%27s%20On_files/yv_global.css Line: 406 Removing "and(min-width:0)" from the selector solves the problem and everything is displayed correctly.
Keywords: testcase
Summary: Flash plugin draws in wrong location → Broken CSS declaration stops parsing of subsequent selectors
Comment 5•16 years ago
|
||
Comment 6•16 years ago
|
||
David, your patch on bug 454226 caused this regression.
Assignee: nobody → dbaron
Blocks: 454226
Updated•16 years ago
|
Summary: Broken CSS declaration stops parsing of subsequent selectors → Broken "@media all" stops parsing of subsequent selectors
Assignee | ||
Updated•16 years ago
|
Assignee: dbaron → nobody
Component: Layout → Style System (CSS)
QA Contact: layout → style-system
Assignee | ||
Comment 7•16 years ago
|
||
Please don't assign bugs to people without their knowledge; that makes it look like they've agreed to work on them, but if they miss the bugmail they might not even know about them.
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → dbaron
Comment 8•16 years ago
|
||
cc'ing adam, as he is writing tests for media query work
Comment 9•16 years ago
|
||
Opening this testcase should display a blue box at the right side of the document. Due to the changes in bug 454226 we stumble over the missing space in the query expression: "and(min-width:0)".
Attachment #345815 -
Attachment is obsolete: true
Comment 10•16 years ago
|
||
Sorry for the bunch of summary updates. Now we should hopefully be fine.
Summary: Broken "@media all" stops parsing of subsequent selectors → Missing space in media query expression stops parsing of subsequent selectors
Assignee | ||
Comment 11•16 years ago
|
||
Hmmm, yeah, the "SkipUntil(',')" in CSSParserImpl::GatherMedia isn't right, since we should really be skipping until ',' or aStopSymbol.
Summary: Missing space in media query expression stops parsing of subsequent selectors → Broken "@media all" stops parsing of subsequent selectors
Assignee | ||
Updated•16 years ago
|
Summary: Broken "@media all" stops parsing of subsequent selectors → error in final media query in list causes media query list to absorb everything until the next comma
Assignee | ||
Comment 12•16 years ago
|
||
Included attachment 345818 [details] as a reftest; ran media queries mochitests (and had to make a fix to them that I should have had to make for bug 454226.
Attachment #345823 -
Flags: superreview?(bzbarsky)
Attachment #345823 -
Flags: review?(bzbarsky)
Updated•16 years ago
|
Flags: in-testsuite?
Updated•16 years ago
|
Attachment #345823 -
Flags: superreview?(bzbarsky)
Attachment #345823 -
Flags: superreview+
Attachment #345823 -
Flags: review?(bzbarsky)
Attachment #345823 -
Flags: review+
Assignee | ||
Comment 13•16 years ago
|
||
Fixed: http://hg.mozilla.org/mozilla-central/rev/01406b3b31c0
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Assignee | ||
Updated•16 years ago
|
Flags: in-testsuite? → in-testsuite+
Assignee | ||
Comment 14•16 years ago
|
||
Also: http://hg.mozilla.org/mozilla-central/rev/b50dbc86eb60
Comment 15•16 years ago
|
||
This seems to have lost us a point in the acid 3 test (test 46 case r) - does the acid test need to be updated or is it a actual regression?
Comment 16•16 years ago
|
||
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081103 Minefield/3.1b2pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081103 Minefield/3.1b2pre. Thanks dbaron.
Status: RESOLVED → VERIFIED
Updated•16 years ago
|
Flags: wanted1.9.1?
Flags: blocking1.9.1?
You need to log in
before you can comment on or make changes to this bug.
Description
•