Bug 76447

Created attachment 122913 [details] Patch v1

Created attachment 122931 [details] Patch v2

Comment on attachment 122931 [details] Patch v2 Attachment 122931 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11282369 New failing tests: css1/text_properties/vertical_align.html css2.1/20110323/background-intrinsic-006.htm svg/custom/object-sizing-width-50p-height-75p-on-target-svg-absolute.xhtml media/video-aspect-ratio.html svg/zoom/page/zoom-svg-through-object-with-auto-size.html css1/box_properties/height.html tables/mozilla_expected_failures/bugs/97619.html svg/custom/object-sizing-width-50p-height-75p-on-target-svg.xhtml svg/custom/object-sizing-width-50p-on-target-svg-absolute.xhtml fast/writing-mode/block-level-images.html tables/mozilla/bugs/bug14929.html tables/mozilla/bugs/bug86708.html svg/custom/object-sizing-width-75p-height-50p-on-target-svg-absolute.xhtml svg/zoom/page/zoom-svg-through-object-with-absolute-size-2.xhtml tables/mozilla/bugs/bug101674.html fast/replaced/table-percent-height.html svg/custom/object-sizing-width-75p-height-50p-on-target-svg.xhtml css1/box_properties/clear.html css1/formatting_model/floating_elements.html css1/box_properties/float.html svg/custom/object-sizing-width-50p-height-50p-on-target-svg-absolute.xhtml css2.1/t1004-c5524-width-00-b-g.html

(In reply to comment #3) > (From update of attachment 122931 [details]) > Attachment 122931 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/11282369 > > New failing tests: > css1/text_properties/vertical_align.html > css2.1/20110323/background-intrinsic-006.htm > svg/custom/object-sizing-width-50p-height-75p-on-target-svg-absolute.xhtml > media/video-aspect-ratio.html > svg/zoom/page/zoom-svg-through-object-with-auto-size.html > css1/box_properties/height.html > tables/mozilla_expected_failures/bugs/97619.html > svg/custom/object-sizing-width-50p-height-75p-on-target-svg.xhtml > svg/custom/object-sizing-width-50p-on-target-svg-absolute.xhtml > fast/writing-mode/block-level-images.html > tables/mozilla/bugs/bug14929.html > tables/mozilla/bugs/bug86708.html > svg/custom/object-sizing-width-75p-height-50p-on-target-svg-absolute.xhtml > svg/zoom/page/zoom-svg-through-object-with-absolute-size-2.xhtml > tables/mozilla/bugs/bug101674.html > fast/replaced/table-percent-height.html > svg/custom/object-sizing-width-75p-height-50p-on-target-svg.xhtml > css1/box_properties/clear.html > css1/formatting_model/floating_elements.html > css1/box_properties/float.html > svg/custom/object-sizing-width-50p-height-50p-on-target-svg-absolute.xhtml > css2.1/t1004-c5524-width-00-b-g.html Hrm, I wonder about changes in non-SVG context :( Not sure whats going on here. I wish there was a way to obtain these results prior to landing :(

Comment on attachment 122931 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=122931&action=review r=me, with a few nits. > Source/WebCore/rendering/RenderBoxModelObject.cpp:882 > + resolvedWidth = static_cast<int>(ceilf(intrinsicWidth.value() * style()->effectiveZoom())); I would prefer the more compact constructor style cast int(ceilf(intrinsicWidth.value() * style()->effectiveZoom())) here and elsewhere. Not sure what our style policy is for this. > Source/WebCore/svg/SVGSVGElement.h:79 > + enum ConsiderCSSMode { > + RespectCSSProperties, > + IgnoreCSSProperties > }; I think it would be better to eliminate this enum... > Source/WebCore/svg/SVGSVGElement.h:83 > + Length intrinsicWidth(ConsiderCSSMode = RespectCSSProperties) const; > + Length intrinsicHeight(ConsiderCSSMode = RespectCSSProperties) const; ...and just have separate functions for the two cases. The code would read better I think.

Committed r105402: <http://trac.webkit.org/changeset/105402>

(In reply to comment #5) > I would prefer the more compact constructor style cast int(ceilf(intrinsicWidth.value() * style()->effectiveZoom())) here and elsewhere. Not sure what our style policy is for this. I left it as-is per IRC discussion. > I think it would be better to eliminate this enum... > ...and just have separate functions for the two cases. The code would read better I think. Fixed. Thanks a lot! I'll go watch the Chromium bots, if there's really as much broken as it says..

(In reply to comment #7) Argl, I ran the full set of tests w/o my patch applied in a wrong git branch. Several text results need rebaselines, I'm going to take care of that now.

Created attachment 123292 [details] Patch v3

Uploading a new version, that should have no regressions on Mac left. Awaiting cr-ews results, to find out whether it only has the failing tests that I also had to rebaseline on Mac.

Ok I'm happy with the results obtained by the cr-ews bot. I updated platform/chromium/test_expectations.txt and trying to land it a second time.

Committed r105513: <http://trac.webkit.org/changeset/105513>

Comment on attachment 123292 [details] Patch v3 Attachment 123292 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11297113 New failing tests: svg/zoom/page/zoom-background-images.html svg/custom/object-sizing-width-50p-height-75p-on-target-svg-absolute.xhtml svg/custom/object-sizing-width-50p-height-50p-on-target-svg-absolute.xhtml tables/mozilla/bugs/bug101674.html tables/mozilla/bugs/bug86708.html svg/custom/object-sizing-width-75p-height-50p-on-target-svg.xhtml svg/custom/object-sizing-width-75p-height-50p-on-target-svg-absolute.xhtml tables/mozilla_expected_failures/bugs/97619.html svg/custom/object-sizing-width-50p-height-75p-on-target-svg.xhtml svg/custom/object-sizing-width-50p-on-target-svg-absolute.xhtml

Landed Gtk rebaselines in r105516.

svg/wicd/rightsizing-grid.xhtml is also broken because of this.

(In reply to comment #15) > svg/wicd/rightsizing-grid.xhtml is also broken because of this. Broken, as in crash? Looks wrong? Just tried again with ToT, and it passes fine on Lion using tolerance 0.

SL bot shows it as flakey: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r105576%20(36564)/results.html

(In reply to comment #17) > SL bot shows it as flakey: > http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r105576%20(36564)/results.html I can reproduce it as well locally, I'll investigate in the morning, it's on top of my list.

(In reply to comment #18) > (In reply to comment #17) > > SL bot shows it as flakey: Closing bug, now that 77099 landed, which fixes the flakiness. Thanks for your patience.

Committed r113436: <http://trac.webkit.org/changeset/113436>