Increase code sharing between CSSComputedStyleDeclaration and CSSPropertyLonghand.
Created attachment 133929 [details] Patch
Comment on attachment 133929 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133929&action=review > Source/WebCore/css/CSSPropertyLonghand.cpp:225 > - CSSPropertyOutlineWidth, > - CSSPropertyOutlineStyle, > CSSPropertyOutlineColor, > - CSSPropertyOutlineOffset > + CSSPropertyOutlineStyle, > + CSSPropertyOutlineWidth This change doesn't look right. border property's values are width, style, and then color, not color, style, then width unless you're traversing backwards.
(In reply to comment #2) > (From update of attachment 133929 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133929&action=review > > > Source/WebCore/css/CSSPropertyLonghand.cpp:225 > > - CSSPropertyOutlineWidth, > > - CSSPropertyOutlineStyle, > > CSSPropertyOutlineColor, > > - CSSPropertyOutlineOffset > > + CSSPropertyOutlineStyle, > > + CSSPropertyOutlineWidth > > This change doesn't look right. border property's values are width, style, and then color, not color, style, then width unless you're traversing backwards. http://www.w3.org/TR/css3-ui/#outline says : [ <‘outline-color’> || <‘outline-style’> || <‘outline-width’> ] | inherit The order is not enforced for sure but we should follow the other browsers and they return that order when possible.
(In reply to comment #3) > http://www.w3.org/TR/css3-ui/#outline says : > > [ <‘outline-color’> || <‘outline-style’> || <‘outline-width’> ] | inherit > > The order is not enforced for sure but we should follow the other browsers and they return that order when possible. Huh, they use a different ordering huh? That's annoying. Can we give a feedback to the CSS WG to match border?
(In reply to comment #4) > (In reply to comment #3) > > http://www.w3.org/TR/css3-ui/#outline says : > > > > [ <‘outline-color’> || <‘outline-style’> || <‘outline-width’> ] | inherit > > > > The order is not enforced for sure but we should follow the other browsers and they return that order when possible. > > Huh, they use a different ordering huh? That's annoying. Can we give a feedback to the CSS WG to match border? Has been like this for a while now. Nobody should rely on the order though but I'm not sure if it will break real use case out there.
(In reply to comment #5) > Has been like this for a while now. Nobody should rely on the order though but I'm not sure if it will break real use case out there. What do other user agents do? Do they follow the spec? Or do they generate in the same order WebKit currently does?
(In reply to comment #6) > (In reply to comment #5) > > Has been like this for a while now. Nobody should rely on the order though but I'm not sure if it will break real use case out there. > > What do other user agents do? Do they follow the spec? Or do they generate in the same order WebKit currently does? When I wrote the patches for shorthand support in CSSComputedStyleDeclaration I tried to make sure we were matching FF and Opera so http://trac.webkit.org/browser/trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-outline-shorthand.html expect that...
(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > Has been like this for a while now. Nobody should rely on the order though but I'm not sure if it will break real use case out there. > > > > What do other user agents do? Do they follow the spec? Or do they generate in the same order WebKit currently does? > > When I wrote the patches for shorthand support in CSSComputedStyleDeclaration I tried to make sure we were matching FF and Opera so http://trac.webkit.org/browser/trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-outline-shorthand.html expect that... Do they match?
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > (In reply to comment #5) > > > > Has been like this for a while now. Nobody should rely on the order though but I'm not sure if it will break real use case out there. > > > > > > What do other user agents do? Do they follow the spec? Or do they generate in the same order WebKit currently does? > > > > When I wrote the patches for shorthand support in CSSComputedStyleDeclaration I tried to make sure we were matching FF and Opera so http://trac.webkit.org/browser/trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-outline-shorthand.html expect that... > > Do they match? FF11 doesn't implement it but Opera returns the same order (modulo the style differences) when I load the test case. FAIL computedStyle.getPropertyValue('outline') should be rgb(255, 0, 0) solid 5px. Was rgb(255, 0, 0) solid 6px.
Comment on attachment 133929 [details] Patch Okay.
Comment on attachment 133929 [details] Patch Attachment 133929 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12146262 New failing tests: fast/css/cssText-shorthand.html
Created attachment 133970 [details] Archive of layout-test-results from ec2-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 134019 [details] Patch for landing
Comment on attachment 134019 [details] Patch for landing Clearing flags on attachment: 134019 Committed r112254: <http://trac.webkit.org/changeset/112254>
All reviewed patches have been landed. Closing bug.