Summary: | Change computeStickyPositionConstraints to use LayoutBoxExtent for margins | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Emil A Eklund <eae> | ||||||
Component: | Layout and Rendering | Assignee: | Emil A Eklund <eae> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | eric, leviw, ojan.autocc, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Emil A Eklund
2013-02-04 15:40:23 PST
Created attachment 186482 [details]
Patch
Comment on attachment 186482 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186482&action=review > Source/WebCore/platform/graphics/LayoutSize.h:75 > + void contract(LayoutUnit width, LayoutUnit height) This seems like the analogue of "shrunkTo" below. We should be consistent, though I understand this is consistent with LayourRect's naming. (In reply to comment #2) > (From update of attachment 186482 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=186482&action=review > > > Source/WebCore/platform/graphics/LayoutSize.h:75 > > + void contract(LayoutUnit width, LayoutUnit height) > > This seems like the analogue of "shrunkTo" below. We should be consistent, though I understand this is consistent with LayourRect's naming. Not quite. shrunkTo shrinks it to the minimum of the current and supplied size. Contract always shrinks it by the supplied delta. (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 186482 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=186482&action=review > > > > > Source/WebCore/platform/graphics/LayoutSize.h:75 > > > + void contract(LayoutUnit width, LayoutUnit height) > > > > This seems like the analogue of "shrunkTo" below. We should be consistent, though I understand this is consistent with LayourRect's naming. > > Not quite. shrunkTo shrinks it to the minimum of the current and supplied size. Contract always shrinks it by the supplied delta. I'm not sure if I follow your point. There's expand (always expands) expandTo (max of the two values) and shrunkTo (min of the two values). The missing value would make sense to be called shrunk, not contract. Being internally consistent to LayoutSize makes more sense to me than matching what LayoutRect uses. (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 186482 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=186482&action=review > > > > > > > Source/WebCore/platform/graphics/LayoutSize.h:75 > > > > + void contract(LayoutUnit width, LayoutUnit height) > > > > > > This seems like the analogue of "shrunkTo" below. We should be consistent, though I understand this is consistent with LayourRect's naming. > > > > Not quite. shrunkTo shrinks it to the minimum of the current and supplied size. Contract always shrinks it by the supplied delta. > > I'm not sure if I follow your point. There's expand (always expands) expandTo (max of the two values) and shrunkTo (min of the two values). The missing value would make sense to be called shrunk, not contract. Being internally consistent to LayoutSize makes more sense to me than matching what LayoutRect uses. Sorry, s/shrunk/shrink/ (In reply to comment #5) Ah, I see what you are saying now. Thanks for clarifying. Renamed contract to shrink. Created attachment 189186 [details]
Patch
Comment on attachment 189186 [details]
Patch
I like it :)
Comment on attachment 189186 [details] Patch Clearing flags on attachment: 189186 Committed r143410: <http://trac.webkit.org/changeset/143410> All reviewed patches have been landed. Closing bug. |