Bug 64919 - Remove remaining uses of CSSPrimitiveValue::computeLengthIntForLength()
Summary: Remove remaining uses of CSSPrimitiveValue::computeLengthIntForLength()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Luke Macpherson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-20 20:03 PDT by Luke Macpherson
Modified: 2011-07-28 18:47 PDT (History)
6 users (show)

See Also:

Attachments
Patch (6.06 KB, patch)
2011-07-20 21:16 PDT, Luke Macpherson 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Macpherson 2011-07-20 20:03:41 PDT 
Remove remaining uses of CSSPrimitiveValue::computeLengthIntForLength()
Comment 1 Luke Macpherson 2011-07-20 21:16:34 PDT 
Created attachment 101551 [details]
Patch
Comment 2 Luke Macpherson 2011-07-25 16:37:17 PDT 
Ping!
Comment 3 Luke Macpherson 2011-07-27 17:28:06 PDT 
Been sitting here for over a week now. Adding dglazkov.
Comment 4 Simon Fraser (smfr) 2011-07-27 18:01:52 PDT 
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?
Comment 5 Luke Macpherson 2011-07-27 18:18:58 PDT 
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.
Comment 6 Darin Adler 2011-07-28 17:38:33 PDT 
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.
Comment 7 WebKit Review Bot 2011-07-28 18:47:04 PDT 
Comment on attachment 101551 [details]
Patch

Clearing flags on attachment: 101551

Committed r91969: <http://trac.webkit.org/changeset/91969>
Comment 8 WebKit Review Bot 2011-07-28 18:47:09 PDT 
All reviewed patches have been landed.  Closing bug.