RESOLVED FIXED 62032
Convert RenderBox::overflowClipRect to IntPoint 
https://bugs.webkit.org/show_bug.cgi?id=62032
Summary Convert RenderBox::overflowClipRect to IntPoint
Emil A Eklund
Reported 2011-06-03 11:46:38 PDT
Ongoing tx, ty to IntPoint conversion
Attachments
Patch (16.58 KB, patch)
2011-06-03 12:36 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (16.62 KB, patch)
2011-06-03 13:17 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (14.52 KB, patch)
2011-06-03 15:05 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch for landing (14.51 KB, patch)
2011-06-03 16:08 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Emil A Eklund
Comment 1 2011-06-03 12:36:02 PDT
Created attachment 95948 [details] Patch
Emil A Eklund
Comment 2 2011-06-03 13:17:47 PDT
Created attachment 95954 [details] Patch Fixed typo in ChangeLog
Eric Seidel (no email)
Comment 3 2011-06-03 13:42:15 PDT
Comment on attachment 95954 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95954&action=review > Source/WebCore/rendering/RenderBox.cpp:1145 > + IntRect clipRect(location + borderTopLeft(), size() - borderSize()); borderSize() = IntSize(borderRight(), borderBottom)? > Source/WebCore/rendering/RenderBoxModelObject.h:95 > + virtual IntSize borderSize() const { return IntSize(borderLeft() + borderRight(), borderTop() + borderBottom()); } Hmmm... Seems we should get this from the borderRect(), I'm not sure borderSize is meaningful. > Source/WebCore/rendering/RenderLayer.h:258 > + IntSize scrollbarSize(OverlayScrollbarSizeRelevancy = IgnoreOverlayScrollbarSize) const; This doesn't make any sense. The vertical and horizontal scrollbars are indepenent. How can we put them both into a single size?
Emil A Eklund
Comment 4 2011-06-03 15:05:40 PDT
Created attachment 95970 [details] Patch
Emil A Eklund
Comment 5 2011-06-03 15:06:41 PDT
Thanks Eric, I agree that the scrollbarSize method was a bit weird. Please take another look and see if you like this approach.
Eric Seidel (no email)
Comment 6 2011-06-03 15:39:26 PDT
Comment on attachment 95970 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95970&action=review LGTM. > Source/WebCore/rendering/RenderBox.cpp:1148 > + clipRect.contract(layer()->verticalScrollbarWidth(relevancy), > + layer()->horizontalScrollbarHeight(relevancy)); I wouldn't have bothered wrapping. 1. cause we have no column limit in webkit. and 2. because as soon as you make it 2 lines, we technically would require { } around the if block (even though the executed code is one line).
Emil A Eklund
Comment 7 2011-06-03 16:08:08 PDT
Created attachment 95981 [details] Patch for landing
WebKit Review Bot
Comment 8 2011-06-04 02:53:49 PDT
Comment on attachment 95981 [details] Patch for landing Clearing flags on attachment: 95981 Committed r88102: <http://trac.webkit.org/changeset/88102>
WebKit Review Bot
Comment 9 2011-06-04 02:53:54 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.