For the following shape definitions: circle(at 50px 50px) circle(closest-side at 50px 50px) ellipse(at 50px 50px) ellipse(closest-side at 50px 50px) The computed style should be circle(at 50px 50px) or ellipse(at 50px 50px), omitting the default radii. The attached test for these 4 cases fails as the default radii is not omitted.
Created attachment 231173 [details] Test case for bug
Created attachment 231837 [details] Patch
Created attachment 231838 [details] Patch
Comment on attachment 231838 [details] Patch Attachment 231838 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4714561223524352 New failing tests: fast/shapes/shape-outside-floats/shape-outside-floats-circle-005.html fast/shapes/shape-outside-floats/shape-outside-floats-circle-003.html
Created attachment 231843 [details] Archive of layout-test-results from webkit-ews-07 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-07 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 231838 [details] Patch Attachment 231838 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5334979986849792 New failing tests: fast/shapes/shape-outside-floats/shape-outside-floats-circle-005.html fast/shapes/shape-outside-floats/shape-outside-floats-circle-003.html
Created attachment 231844 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 231852 [details] Patch
Created attachment 231853 [details] Patch
Comment on attachment 231853 [details] Patch Attachment 231853 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4722087885275136 New failing tests: media/W3C/video/networkState/networkState_during_loadstart.html
Created attachment 231858 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 231853 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231853&action=review > Source/WebCore/css/CSSBasicShapes.cpp:117 > + String radius = m_radius ? m_radius->cssText() : String(); > + if (m_radius && m_radius->getValueID() == CSSValueClosestSide) You should just get rid of the ternary and make this a set of if statements. (for example here you get the cssText even if you're just going to throw it away.) > Source/WebCore/css/CSSBasicShapes.cpp:184 > + bool isRadiusXCSSValueClosestSide = m_radiusX->getValueID() == CSSValueClosestSide; > + bool isRadiusYCSSValueClosestSide = m_radiusY && m_radiusY->getValueID() == CSSValueClosestSide; > + if ((!isRadiusXCSSValueClosestSide) || (isRadiusXCSSValueClosestSide && m_radiusY && !isRadiusYCSSValueClosestSide)) I think you should change the name of these booleans to shouldSerializeRadusXValue (m_radiusX->getValueID() != CSSValueClosestSide) and shouldSerializeRadiusYValue (m_radiusY && m_radiusY->getValueID() != CSSValueClosestSide). Then you won't need as many negations in your if statements. You also won't need to check m_radiusY in both setting the boolean and in the if statements, which I found rather confusing. > Source/WebCore/css/CSSBasicShapes.cpp:186 > + if (m_radiusY && !isRadiusYCSSValueClosestSide) Ditto.
Created attachment 232153 [details] Patch Comments are addressed in the patch.
Attachment 232153 [details] did not pass style-queue: ERROR: Source/WebCore/css/CSSBasicShapes.cpp:186: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/css/CSSBasicShapes.cpp:187: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/css/CSSBasicShapes.cpp:188: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 3 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 232155 [details] Patch All comments are addressed in this patch.
Created attachment 232156 [details] Patch All comments are addressed in this patch.
Created attachment 232157 [details] Patch
Comment on attachment 232157 [details] Patch Clearing flags on attachment: 232157 Committed r169406: <http://trac.webkit.org/changeset/169406>
All reviewed patches have been landed. Closing bug.