RESOLVED FIXED 77914
Update LayoutUnit usage in ColumnInfo and RenderFrameSet
https://bugs.webkit.org/show_bug.cgi?id=77914
Summary Update LayoutUnit usage in ColumnInfo and RenderFrameSet
Levi Weintraub
Reported 2012-02-06 16:58:43 PST
There are a handfull of functions and variables that should be switched to LayoutUnits from integers.
Attachments
Patch (3.77 KB, patch)
2012-02-06 17:03 PST, Levi Weintraub
no flags
Patch (3.83 KB, patch)
2012-02-07 10:13 PST, Levi Weintraub
no flags
Levi Weintraub
Comment 1 2012-02-06 17:03:19 PST
Eric Seidel (no email)
Comment 2 2012-02-06 18:19:40 PST
Comment on attachment 125732 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125732&action=review > Source/WebCore/rendering/ColumnInfo.h:76 > + LayoutUnit forcedBreaks() const { return m_forcedBreaks; } Is this a count?
Levi Weintraub
Comment 3 2012-02-07 10:13:31 PST
Levi Weintraub
Comment 4 2012-02-07 10:14:50 PST
(In reply to comment #2) > (From update of attachment 125732 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=125732&action=review > > > Source/WebCore/rendering/ColumnInfo.h:76 > > + LayoutUnit forcedBreaks() const { return m_forcedBreaks; } > > Is this a count? Good catch! I had the functions swapped.
Eric Seidel (no email)
Comment 5 2012-02-07 14:16:04 PST
Comment on attachment 125869 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125869&action=review > Source/WebCore/ChangeLog:8 > + Updating usage of LayoutUnits in ColumnInfo and RenderFrameSet. You should clarify in your Changelog (or code) why these are different from table columns. All these changes are fine, but I think you should spend a bit more effort on the *why* explanations. :)
Levi Weintraub
Comment 6 2012-02-07 14:17:03 PST
Comment on attachment 125869 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125869&action=review >> Source/WebCore/ChangeLog:8 >> + Updating usage of LayoutUnits in ColumnInfo and RenderFrameSet. > > You should clarify in your Changelog (or code) why these are different from table columns. > > All these changes are fine, but I think you should spend a bit more effort on the *why* explanations. :) Will do. Thanks for the review!
Levi Weintraub
Comment 7 2012-02-07 16:52:44 PST
Levi Weintraub
Comment 8 2012-02-07 16:56:27 PST
Comment on attachment 125869 [details] Patch Clearing Flags
Note You need to log in before you can comment on or make changes to this bug.