Summary: | REGRESSION: Text of entire page incorrectly center aligned | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Elliott Sprehn <esprehn> | ||||||||||||
Component: | CSS | Assignee: | Kevin Ollivier <kevino> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | bdakin, codecolorist, hyatt, webkit | ||||||||||||
Priority: | P1 | Keywords: | HasReduction, Regression | ||||||||||||
Version: | 523.x (Safari 3) | ||||||||||||||
Hardware: | Mac | ||||||||||||||
OS: | OS X 10.4 | ||||||||||||||
URL: | http://www.ffconsultancy.com/languages/ray_tracer/comparison.html | ||||||||||||||
Attachments: |
|
Description
Elliott Sprehn
2008-01-16 18:22:43 PST
This is a regression from Safari 2.0.4 (419.3) with original WebKit on Mac OS X 10.4.11 (8S165). Confirmed with a local debug build of WebKit r29623 with Safari 3.0.4 (523.12.2) on 10.4.11. Unfortunately this bug reproduces with the earliest known WebKit nightly build (r11976), so bisect-builds isn't going to help us. Created attachment 18542 [details] Reduced test case The issue is that one of the stylesheets included in the original page (http://www.ffconsultancy.com/style.css) has an invalid rule, which causes WebKit to stop parsing it, while Firefox 2.0.0.x and Opera 9.2x ignore the rule and continue parsing. The invalid rule is: width: *; When both copies of the invalid rule are removed or commented out, the page renders like Firefox. Created attachment 18543 [details]
Test case variation 1
Text is green in Firefox and Opera.
Created attachment 18544 [details]
Test case variation 2
Text is green in Firefox and Opera.
kevino, who is assigned to this bug? Created attachment 19476 [details]
Fix for the problem that ignores rules with an invalid value
This is my first foray into CSSGrammar.y and Bison code, so I would really appreciate some extra eyes on this fix. I'm not seeing any new layout test failures after the change, but I don't know if there are any corner cases I'm missing or if the approach might be overly general.
Also, I assume we want a layout test created for this, so that the regression won't recur later on? Where should I put it? And do we need to generate results on all platforms for it? (New to writing LayoutTests too, sorry... ;-)
This looks good to me. Should attach test cases to the patch. (In reply to comment #7) > Also, I assume we want a layout test created for this, so that the regression > won't recur later on? Where should I put it? And do we need to generate results > on all platforms for it? (New to writing LayoutTests too, sorry... ;-) I would put the test(s) in LayoutTests/fast/css/. If you make it a text-based test (using window.layoutTestController.dumpAsText()), then you don't have to worry about generating results for every platform. Look in LayoutTests/fast/css for examples that use layoutTestController.dumpAsText() and craft a similar test based on one or more reductions in this bug. I usually prefer to use the js-test-* assertions instead of custom "logging" solutions like getComputedStyle-zIndex-auto.html. See legacy-opacity-styles.html for one example. Darin likes to use the WebKitTools/Scripts/make-js-test-wrappers script to create the resources/*.js file first, then let the script generate the test file for it. Created attachment 19486 [details]
Version of fix with layout test added
Comment on attachment 19486 [details]
Version of fix with layout test added
r=me
Landed in r30704, thanks! *** Bug 161899 has been marked as a duplicate of this bug. *** |