-webkit-margin-collapse: separate doesn't work correctly for before margins

[Andrei Bucur]

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.

[Andrei Bucur]

Created attachment 189523 [details]
Patch

[Dave Hyatt]

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]

Created attachment 189543 [details]
Patch for landing

[WebKit Review Bot]

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]

Created attachment 189546 [details]
Patch for landing v2

Fix the build error.

[WebKit Review Bot]

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]

All reviewed patches have been landed.  Closing bug.