WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
64919
Remove remaining uses of CSSPrimitiveValue::computeLengthIntForLength()
https://bugs.webkit.org/show_bug.cgi?id=64919
Summary
Remove remaining uses of CSSPrimitiveValue::computeLengthIntForLength()
Luke Macpherson
Reported
2011-07-20 20:03:41 PDT
Remove remaining uses of CSSPrimitiveValue::computeLengthIntForLength()
Attachments
Patch
(6.06 KB, patch)
2011-07-20 21:16 PDT
,
Luke Macpherson
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Luke Macpherson
Comment 1
2011-07-20 21:16:34 PDT
Created
attachment 101551
[details]
Patch
Luke Macpherson
Comment 2
2011-07-25 16:37:17 PDT
Ping!
Luke Macpherson
Comment 3
2011-07-27 17:28:06 PDT
Been sitting here for over a week now. Adding dglazkov.
Simon Fraser (smfr)
Comment 4
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?
Luke Macpherson
Comment 5
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.
Darin Adler
Comment 6
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.
WebKit Review Bot
Comment 7
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
>
WebKit Review Bot
Comment 8
2011-07-28 18:47:09 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug