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

Issue 2773453004: Fix block offset used by absolute-positioned objects (Closed)

Created:
3 years, 9 months ago by Gleb Lanbin
Modified:
3 years, 9 months ago
Reviewers:
atotic, kojii, ikilpatrick
CC:
atotic+reviews_chromium.org, blink-reviews, blink-reviews-layout_chromium.org, cbiesinger, chromium-reviews, dgrogan+ng_chromium.org, eae+blinkwatch, glebl+reviews_chromium.org, jchaffraix+rendering, leviw+renderwatch, ojan+watch_chromium.org, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix block offset used by absolute-positioned objects Absolute-positioned objects can be positioned as static if top/bottom is auto. That's why they need to know their block offset that composes from content_size and current MarginStrut. List of changes: 1) Set the correct block offset for absolute-positioned objects 2) Do not reset text child's margin as we need them to set MarginStrut properly. 3) Move ComputeMinMaxContentSize and Style methods definition to the base class and make them to be pure virtual methods. 4) Change CalculateMargins to work with NGLayoutInputNode base class 5) Update TestExpectations BUG=635619 TEST=virtual/layout_ng/fast/block/margin-collapse/002.html and others Review-Url: https://codereview.chromium.org/2773453004 Cr-Commit-Position: refs/heads/master@{#459499} Committed: https://chromium.googlesource.com/chromium/src/+/6d8a607a3ba62cbd82d8ada7b10ad5c1ef5c978c

Patch Set 1 #

Total comments: 5

Patch Set 2 : add DCHECK #

Patch Set 3 : git rebase-update #

Patch Set 4 : git rebase-update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -154 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 22 chunks +55 lines, -118 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_bidi_paragraph.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_bidi_paragraph.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.h View 2 chunks +1 line, -5 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc View 1 2 3 4 chunks +13 lines, -15 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_node.h View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_inline_node.h View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_inline_node.cc View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_layout_input_node.h View 1 2 3 3 chunks +10 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_line_box_fragment_builder.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_text_fragment_builder.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 41 (33 generated)
Gleb Lanbin
3 years, 9 months ago (2017-03-22 23:47:24 UTC) #4
atotic
https://codereview.chromium.org/2773453004/diff/20001/third_party/WebKit/LayoutTests/TestExpectations File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2773453004/diff/20001/third_party/WebKit/LayoutTests/TestExpectations#newcode336 third_party/WebKit/LayoutTests/TestExpectations:336: #### Passed: 5 Verbal with gleb: we lost one ...
3 years, 9 months ago (2017-03-23 00:14:46 UTC) #8
ikilpatrick
https://codereview.chromium.org/2773453004/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc (right): https://codereview.chromium.org/2773453004/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc#newcode315 third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:315: if (child->Style().clear() != EClear::kNone) { what is the value ...
3 years, 9 months ago (2017-03-23 16:29:05 UTC) #12
Gleb Lanbin
https://codereview.chromium.org/2773453004/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc (right): https://codereview.chromium.org/2773453004/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc#newcode315 third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:315: if (child->Style().clear() != EClear::kNone) { On 2017/03/23 16:29:04, ikilpatrick ...
3 years, 9 months ago (2017-03-23 17:47:16 UTC) #13
ikilpatrick
lgtm, kojii@ can you check the inline changes? Want to make sure you are ok ...
3 years, 9 months ago (2017-03-23 18:02:54 UTC) #18
kojii
lgtm, thank you for doing this. Handling margin collapsing to anonymous block was something I ...
3 years, 9 months ago (2017-03-24 02:55:19 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2773453004/80001
3 years, 9 months ago (2017-03-24 18:30:35 UTC) #38
commit-bot: I haz the power
3 years, 9 months ago (2017-03-24 18:56:44 UTC) #41
Message was sent while issue was closed.
Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/6d8a607a3ba62cbd82d8ada7b10a...

Powered by Google App Engine
This is Rietveld 408576698