Bug 135162 – Merge FixedIntegerConversion and FixedFloatConversion for CSSPrimitiveValue::convertToLength

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: CSS

[Martin Hodovan]

Merge FixedIntegerConversion and FixedFloatConversion for CSSPrimitiveValue::convertToLength

[Martin Hodovan]

Created attachment 235296 [details]
Proposed patch

Patch by Timothy Loh, backported from Blink: https://codereview.chromium.org/68203023

[Build Bot]

Comment on attachment 235296 [details]
Proposed patch

Attachment 235296 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6545296393764864

New failing tests:
media/W3C/video/paused/paused_false_during_play.html
transforms/cssmatrix-2d-interface.xhtml

[Build Bot]

Created attachment 235324 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Darin Adler]

Comment on attachment 235296 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235296&action=review

> Source/WebCore/ChangeLog:14
> +        differences. The FixedIntegerConversion clamps to about the range of
> +        LayoutUnits (+-32mil) and doesn't actually convert to an integer.
> +        FixedFloatConversion simple doesn't clamp.

This describes a difference.

> Source/WebCore/ChangeLog:23
> +        No change in functionality, no new tests.

This claims there is no change. Which is it?

[Brent Fulgham]

(In reply to comment #4)
> > Source/WebCore/ChangeLog:23
> > +        No change in functionality, no new tests.
> 
> This claims there is no change. Which is it?

The fact that it introduced test failures indicates there is a change! :-)

[Brent Fulgham]

Comment on attachment 235296 [details]
Proposed patch

This change doesn't apply against current sources, and introduces at least one test regression. Please rebaseline the patch against current sources and correct the test failure.

[Brent Fulgham]

This code has been significantly refactored since this patch was proposed. There doesn't seem to be any action we can take here.