| Summary: | Prepare html classes for sub-pixel LayoutUnits | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Levi Weintraub <leviw> | ||||||||
| Component: | Layout and Rendering | Assignee: | Levi Weintraub <leviw> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | eae, eric.carlson, eric, feature-media-reviews, jchaffraix, tkent, webkit.review.bot | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 60318 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Levi Weintraub
2012-04-09 11:40:24 PDT
Created attachment 136279 [details]
Patch
Comment on attachment 136279 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136279&action=review > Source/WebCore/html/ValidationMessage.cpp:123 > + bubble->setInlineStyleProperty(CSSPropertyTop, hostY + static_cast<double>(hostRect.height()), CSSPrimitiveValue::CSS_PX); Why the double casts? Comment on attachment 136279 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136279&action=review >> Source/WebCore/html/ValidationMessage.cpp:123 >> + bubble->setInlineStyleProperty(CSSPropertyTop, hostY + static_cast<double>(hostRect.height()), CSSPrimitiveValue::CSS_PX); > > Why the double casts? You're right, this can be removed. We were just missing a double flavor of operator+ in FractionalLayoutUnit. Will upload again. Created attachment 136295 [details]
Patch
Comment on attachment 136295 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136295&action=review > Source/WebCore/platform/Length.h:127 > + // FIXME: When we switch to sub-pixel layout, value will return float by default, and this will > + // no longer simply return value(). > + int intValue() const > + { > + return value(); > + } > + What will it return? static_cast<int>(value())? round(value())? Can we make it return whatever that is now? Or add such in a comment or something? Comment on attachment 136295 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136295&action=review >> Source/WebCore/platform/Length.h:127 >> + > > What will it return? static_cast<int>(value())? round(value())? Can we make it return whatever that is now? Or add such in a comment or something? Sorry, my comment was better in the changelog than the code. This will simply inherit the current logic of value(). Created attachment 136299 [details]
Patch
I hope I've cleared up the issues you called out. I'd love another look :) Comment on attachment 136299 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136299&action=review > Source/WebCore/platform/Length.h:122 > + // FIXME: When we switch to sub-pixel layout, value will return float by default, and this will inherit > + // the current implementation of value(). Were you going to change this? Comment on attachment 136299 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136299&action=review >> Source/WebCore/platform/Length.h:122 >> + // the current implementation of value(). > > Were you going to change this? Oh, I guess you did. Anyway, the switch is very soon, so this is fine regardless. Comment on attachment 136299 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136299&action=review >>> Source/WebCore/platform/Length.h:122 >>> + // the current implementation of value(). >> >> Were you going to change this? > > Oh, I guess you did. Anyway, the switch is very soon, so this is fine regardless. Thanks for the review! Comment on attachment 136299 [details] Patch Clearing flags on attachment: 136299 Committed r113645: <http://trac.webkit.org/changeset/113645> All reviewed patches have been landed. Closing bug. |