Bug 46938 - Rewrite margin collapsing to be block-flow-aware
Summary: Rewrite margin collapsing to be block-flow-aware
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks: 46123
  Show dependency treegraph
 
Reported: 2010-09-30 14:36 PDT by Dave Hyatt
Modified: 2010-09-30 15:24 PDT (History)
1 user (show)

See Also:


Attachments
Patch (57.26 KB, patch)
2010-09-30 14:39 PDT, Dave Hyatt
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2010-09-30 14:36:13 PDT
Rewrite margin collapsing to be block-flow-aware.
Comment 1 Dave Hyatt 2010-09-30 14:39:27 PDT
Created attachment 69385 [details]
Patch
Comment 2 WebKit Review Bot 2010-09-30 14:44:12 PDT
Attachment 69385 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/rendering/RenderBlock.h:190:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Sam Weinig 2010-09-30 14:48:14 PDT
Comment on attachment 69385 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=69385&action=review

> WebCore/rendering/RenderBlock.h:190
> +    struct MarginValues
> +    {

The { should be on the same line as struct MarginValues

> WebCore/rendering/RenderBlock.h:212
> +        
> +        int beforePos() const { return m_beforePos; }
> +        int beforeNeg() const { return m_beforeNeg; }
> +        int afterPos() const { return m_afterPos; }
> +        int afterNeg() const { return m_afterNeg; }
> +        
> +        void setBeforePos(int pos) { m_beforePos = pos; }
> +        void setBeforeNeg(int neg) { m_beforeNeg = neg; }
> +        void setAfterPos(int pos) { m_afterPos = pos; }
> +        void setAfterNeg(int neg) { m_afterNeg = neg; }
> +
> +        int m_beforePos;
> +        int m_beforeNeg;
> +        int m_afterPos;
> +        int m_afterNeg;
> +    };

What is the purpose of these getter and setters. can we just remove the m_ from the members and remove the functions?


I think replacing neg with negative and pos with positive would make this code read better.
Comment 4 Dave Hyatt 2010-09-30 15:24:43 PDT
Fixed in r68842.