Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(57)

Issue 10578056: Merge 120859 - REGRESSION(r113885): Margin not properly applied to elements with align=center (Closed)

Created:
8 years, 6 months ago by Julien - ping for review
Modified:
8 years, 6 months ago
Reviewers:
jchaffraix
CC:
chromium-reviews
Base URL:
http://svn.webkit.org/repository/webkit/branches/chromium/1180/
Visibility:
Public.

Description

Merge 120859 - REGRESSION(r113885): Margin not properly applied to elements with align=center https://bugs.webkit.org/show_bug.cgi?id=89515 Reviewed by Levi Weintraub. Source/WebCore: Tests: fast/block/negative-margin-start-positive-margin-end.html fast/block/negative-start-margin-align-center.html fast/block/positive-margin-block-child-align-center-rtl.html fast/block/positive-margin-block-child-align-center.html fast/block/positive-margin-start-align-center.html fast/block/positive-margin-start-negative-margin-end-align-center.html fast/table/table-cell-negative-start-margin-align-center.html r113885 changed the code-path for elements with auto width to call computeInlineDirectionMargins. However this uncovered an existing bug in the function when dealing with align="center" (text-align: -webkit-center) where we would ignore the margin. This goes against what other browsers are doing and our previous behavior. Note that align="left" and "right" are likely impacted too and will be investigated / fixed in follow-up changes. * rendering/RenderBox.cpp: (WebCore::RenderBox::computeInlineDirectionMargins): To match other browsers' behavior, changed the function to include margin in our computations. LayoutTests: * fast/block/negative-margin-start-positive-margin-end-expected.html: Added. * fast/block/negative-margin-start-positive-margin-end.html: Added. * fast/block/negative-start-margin-align-center-expected.html: Added. * fast/block/negative-start-margin-align-center.html: Added. * fast/block/positive-margin-block-child-align-center-expected.html: Added. * fast/block/positive-margin-block-child-align-center.html: Added. * fast/block/positive-margin-start-align-center-expected.html: Added. * fast/block/positive-margin-start-align-center.html: Added. * fast/block/positive-margin-start-negative-margin-end-align-center-expected.html: Added. * fast/block/positive-margin-start-negative-margin-end-align-center.html: Added. Those checks the combination of margin start / end both positive and negative. * fast/block/positive-margin-block-child-align-center-rtl-expected.html: Added. * fast/block/positive-margin-block-child-align-center-rtl.html: Added. One ltr test as I didn't find any. * fast/table/table-cell-negative-start-margin-align-center-expected.html: Added. * fast/table/table-cell-negative-start-margin-align-center.html: Added. This test is very similar to the one above but involves a table. TBR=jchaffraix@webkit.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=120872

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+-8 lines, --12 lines) Patch
A + LayoutTests/fast/block/negative-margin-start-positive-margin-end.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/negative-margin-start-positive-margin-end-expected.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/negative-start-margin-align-center.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/negative-start-margin-align-center-expected.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/positive-margin-block-child-align-center.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/positive-margin-block-child-align-center-expected.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/positive-margin-block-child-align-center-rtl.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/positive-margin-block-child-align-center-rtl-expected.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/positive-margin-start-align-center.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/positive-margin-start-align-center-expected.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/block/positive-margin-start-negative-margin-end-align-center-expected.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/table/table-cell-negative-start-margin-align-center.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + LayoutTests/fast/table/table-cell-negative-start-margin-align-center-expected.html View 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/WebCore/rendering/RenderBox.cpp View 1 chunk +6 lines, -2 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
Julien - ping for review
8 years, 6 months ago (2012-06-20 22:22:20 UTC) #1

          

Powered by Google App Engine
This is Rietveld 408576698