Bug 80196 - Replace uses of x(), y() and width(), height() pairs with locationOffset and size()
Summary: Replace uses of x(), y() and width(), height() pairs with locationOffset and ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Emil A Eklund
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-02 15:11 PST by Emil A Eklund
Modified: 2012-03-05 14:59 PST (History)
7 users (show)

See Also:


Attachments
Patch (11.44 KB, patch)
2012-03-02 15:47 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch for landing (12.79 KB, patch)
2012-03-05 12:03 PST, Emil A Eklund
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emil A Eklund 2012-03-02 15:11:04 PST
We still have a handful of places in the code where we use x, y and width, height pairs instead of Points and Sizes.
Comment 1 Emil A Eklund 2012-03-02 15:47:37 PST
Created attachment 129970 [details]
Patch
Comment 2 Julien Chaffraix 2012-03-05 10:42:18 PST
Comment on attachment 129970 [details]
Patch

Nit: In HTMLCanvasElement, this line could welcome the same change: FloatRect r = mapRect(rect, FloatRect(0, 0, size().width(), size().height()), destRect);

Also there are other places where we could use the same pattern (like IntRect(0, 0, someComputedHeight, someComputedWidth) in the rendering code) so it seems as if the title is misleading IMHO.
Comment 3 Emil A Eklund 2012-03-05 10:44:39 PST
Thanks! I'll make the suggested changes to HTMLCanvasElement and have renamed the bug to make it clear what I am changing.
Comment 4 Julien Chaffraix 2012-03-05 10:52:20 PST
(In reply to comment #3)
> Thanks! I'll make the suggested changes to HTMLCanvasElement and have renamed the bug to make it clear what I am changing.

It makes it a little bit clearer but what I had in mind was not to use 'remaining' as there are some remaining use after your change (look at RenderScrollbar.cpp for example).
Comment 5 Emil A Eklund 2012-03-05 10:58:11 PST
Good point. Grep failed to find some of the computed value ones. I'll take care of the ones in RenderScrollbar and have updated the summary as suggested.
Comment 6 Emil A Eklund 2012-03-05 12:03:44 PST
Created attachment 130174 [details]
Patch for landing
Comment 7 Eric Seidel (no email) 2012-03-05 13:45:11 PST
Comment on attachment 130174 [details]
Patch for landing

These changes are so fantastic.  Thank you.
Comment 8 WebKit Review Bot 2012-03-05 14:59:11 PST
Comment on attachment 130174 [details]
Patch for landing

Clearing flags on attachment: 130174

Committed r109805: <http://trac.webkit.org/changeset/109805>
Comment 9 WebKit Review Bot 2012-03-05 14:59:16 PST
All reviewed patches have been landed.  Closing bug.