[webkit-reviews] review denied: [Bug 48110] getBoundingClientRect: Do not truncate the coordinates to integers : [Attachment 72622] Fix and layout test
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 2 00:15:37 PDT 2010
Nikolas Zimmermann <zimmermann at kde.org> has denied Cosmin Truta
<ctruta at chromium.org>'s request for review:
Bug 48110: getBoundingClientRect: Do not truncate the coordinates to integers
https://bugs.webkit.org/show_bug.cgi?id=48110
Attachment 72622: Fix and layout test
https://bugs.webkit.org/attachment.cgi?id=72622&action=review
------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=72622&action=review
This looks great, especially the tests, though some nitpicks:
> WebCore/rendering/RenderObject.h:1006
> +inline float adjustFloatForAbsoluteZoom(float value, RenderObject* renderer)
> {
> - rect.setX(adjustForAbsoluteZoom(rect.x(), renderer));
> - rect.setY(adjustForAbsoluteZoom(rect.y(), renderer));
> - rect.setWidth(adjustForAbsoluteZoom(rect.width(), renderer));
> - rect.setHeight(adjustForAbsoluteZoom(rect.height(), renderer));
> + return adjustFloatForAbsoluteZoom(value, renderer->style());
> }
This helper function seems useless.
> WebCore/rendering/RenderObject.h:1030
> + rect.setX(adjustFloatForAbsoluteZoom(rect.x(), renderer));
> + rect.setY(adjustFloatForAbsoluteZoom(rect.y(), renderer));
> + rect.setWidth(adjustFloatForAbsoluteZoom(rect.width(), renderer));
> + rect.setHeight(adjustFloatForAbsoluteZoom(rect.height(), renderer));
Just cache the RenderStyle* style = renderer->style() here, and pass it instead
of renderer to adjustFloatForAbsoluteZoom.
> WebCore/rendering/style/RenderStyle.h:1322
> + float zoomFactor = style->effectiveZoom();
> + return value / zoomFactor;
No need for this local variable zoomFactor.
More information about the webkit-reviews
mailing list