Bug 109956

Summary: -webkit-margin-collapse: separate doesn't work correctly for before margins
Product: WebKit Reporter: Andrei Bucur <abucur>
Component: WebCore Misc.Assignee: Andrei Bucur <abucur>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, esprehn+autocc, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing
webkit.review.bot: commit-queue-
Patch for landing v2 none

Andrei Bucur
Reported 2013-02-15 11:16:07 PST
Specifying -webkit-margin-collapse doesn't work correctly in the case of child-container adjoining margins. At the before side of the block, the child is positioned at the sum of the margins if it specifies separate. This is wrong: the block and the child should be placed at their own margin values inside their containers.
Attachments
Patch (6.52 KB, patch)
2013-02-21 06:52 PST, Andrei Bucur
no flags
Patch for landing (6.46 KB, patch)
2013-02-21 09:06 PST, Andrei Bucur
webkit.review.bot: commit-queue-
Patch for landing v2 (6.47 KB, patch)
2013-02-21 09:26 PST, Andrei Bucur
no flags
Andrei Bucur
Comment 1 2013-02-21 06:52:48 PST
Dave Hyatt
Comment 2 2013-02-21 08:51:41 PST
Comment on attachment 189523 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189523&action=review r=me > Source/WebCore/rendering/RenderBlock.cpp:2079 > + if (marginInfo.canCollapseWithMarginBefore()) > + setLogicalHeight(logicalHeight() + marginBeforeForChild(child)); > + else > + setLogicalHeight(logicalHeight() + marginInfo.margin() + marginBeforeForChild(child)); I think this would a bit better as: LayoutUnit separateMargin = !marginInfo.canCollapseWithMarginBefore() ? marginInfo.margin() : 0; setLogicalHeight(logicalHeight() + marginBeforeForChild(child) + separateMargin);
Andrei Bucur
Comment 3 2013-02-21 09:06:17 PST
Created attachment 189543 [details] Patch for landing
WebKit Review Bot
Comment 4 2013-02-21 09:12:33 PST
Comment on attachment 189543 [details] Patch for landing Rejecting attachment 189543 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: y-inlines-hidden -Wsign-compare -c ../../Source/WebCore/rendering/RenderBlock.cpp -o obj/Source/WebCore/rendering/webcore_rendering.RenderBlock.o ../../Source/WebCore/rendering/RenderBlock.cpp: In member function 'WebCore::LayoutUnit WebCore::RenderBlock::collapseMargins(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&)': ../../Source/WebCore/rendering/RenderBlock.cpp:2084: error: operands to ?: have different types 'WebCore::LayoutUnit' and 'int' ninja: build stopped: subcommand failed. Full output: http://queues.webkit.org/results/16701044
Andrei Bucur
Comment 5 2013-02-21 09:26:16 PST
Created attachment 189546 [details] Patch for landing v2 Fix the build error.
WebKit Review Bot
Comment 6 2013-02-21 09:43:53 PST
Comment on attachment 189546 [details] Patch for landing v2 Clearing flags on attachment: 189546 Committed r143617: <http://trac.webkit.org/changeset/143617>
WebKit Review Bot
Comment 7 2013-02-21 09:43:56 PST
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.