Bug 64919 – Remove remaining uses of CSSPrimitiveValue::computeLengthIntForLength()

• Component: New Bugs
• Status: RESOLVED FIXED

[Luke Macpherson]

Remove remaining uses of CSSPrimitiveValue::computeLengthIntForLength()

[Luke Macpherson]

Created attachment 101551 [details]
Patch

[Luke Macpherson]

Ping!

[Luke Macpherson]

Been sitting here for over a week now. Adding dglazkov.

[Simon Fraser (smfr)]

Comment on attachment 101551 [details]
Patch

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

> Source/WebCore/css/CSSPrimitiveValue.cpp:279
> +    // FIXME: Length.h no longer expects 28 bit integers, so these bounds should be INT_MAX and INT_MIN

Why not fix that now?

[Luke Macpherson]

Comment on attachment 101551 [details]
Patch

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

>> Source/WebCore/css/CSSPrimitiveValue.cpp:279
>> +    // FIXME: Length.h no longer expects 28 bit integers, so these bounds should be INT_MAX and INT_MIN
> 
> Why not fix that now?

Trying to avoid behavioral changes in this patch. Changing this will impact a lot of properties, and potentially breaks a number of tests. This comment is mostly about correcting the obsolete one above.

[Darin Adler]

Comment on attachment 101551 [details]
Patch

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

>>> Source/WebCore/css/CSSPrimitiveValue.cpp:279
>>> +    // FIXME: Length.h no longer expects 28 bit integers, so these bounds should be INT_MAX and INT_MIN
>> 
>> Why not fix that now?
> 
> Trying to avoid behavioral changes in this patch. Changing this will impact a lot of properties, and potentially breaks a number of tests. This comment is mostly about correcting the obsolete one above.

Probably the word is “could” rather than “should”.

Also would be best to have a period instead.

> Source/WebCore/platform/Length.h:97
> +    void setQuirk(bool quirk)
> +    {
> +        m_quirk = quirk;
> +    }

I find the name “quirk” puzzling for a boolean data member. A boolean is not a quirk. Not your fault, but really confusing.

[WebKit Review Bot]

Comment on attachment 101551 [details]
Patch

Clearing flags on attachment: 101551

Committed r91969: <http://trac.webkit.org/changeset/91969>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.