Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2887623002:
[LayoutNG] Place InlineFlowBox from NGFragment tree (Closed)
|
Created:
3 years, 7 months ago by kojii Modified:
3 years, 7 months ago 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[LayoutNG] Place InlineFlowBox from NGFragment tree
NGInlineNode::CopyFragmentDataToLayoutBox copies geometry from inline
NGFragment tree to RootInlineBox and InlineTextBox, but didn't include
InlineFlowBox. This results in InlineFlowBox, generated for nested
inline elements, to have (0,0). Their children are placed, but since
parent InlineFlowBox is at (0,0), they were clipped.
This fix turns several tests to fail. They used to pass because expected
images did not render. This fix unhides such tests.
BUG=636993, 723117
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Review-Url: https://codereview.chromium.org/2887623002
Cr-Commit-Position: refs/heads/master@{#473444}
Committed: https://chromium.googlesource.com/chromium/src/+/1dd63993f535aa99d4f0ecb3159751b5fcb2089f
Patch Set 1 #Patch Set 2 : Fix #Patch Set 3 : #Patch Set 4 : Rebase #
Total comments: 1
Patch Set 5 : Update TestExpectations #
Messages
Total messages: 34 (29 generated)
Description was changed from ========== ngflowbox BUG= ========== to ========== ngflowbox BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng ==========
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== ngflowbox BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng ========== to ========== [LayoutNG] Place InlineFlowBox from NGFragment tree NGInlineNode::CopyFragmentDataToLayoutBox copies geometry from inline NGFragment tree to RootInlineBox and InlineTextBox, but didn't include InlineFlowBox. This results in InlineFlowBox, generated for nested inline elements, to have (0,0). Their children are placed, but since parent InlineFlowBox is at (0,0), they were clipped. This fix turns several tests to fail. They used to pass because expected images did not render. This fix unhides such tests. BUG=636993, 723117 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng ==========
kojii@chromium.org changed reviewers: + eae@chromium.org
PTAL. Contacted gleb@ offline and he thinks new failing tests are ok, filed crbug.com/723117.
kojii@chromium.org changed reviewers: + glebl@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Patchset #6 (id:100001) has been deleted
Patchset #5 (id:80001) has been deleted
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #4 (id:60001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_layout_tests_layout_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?))
lgtm https://codereview.chromium.org/2887623002/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2887623002/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/TestExpectations:354: crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats-clear/floats-124.xht [ Skip ] .optional you should use crbug.com723117 here. .optional2 I already landed my fix if you rebase and re-run test they should pass.
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/05/18 at 23:41:06, glebl wrote: > .optional2 I already landed my fix if you rebase and re-run test they should pass. Done in PS5, thank you!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by kojii@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from glebl@chromium.org Link to the patchset: https://codereview.chromium.org/2887623002/#ps140001 (title: "Update TestExpectations")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 140001, "attempt_start_ts": 1495303696618100,
"parent_rev": "e294988760e65f29ca5e8ac8ae56f71fd8e514de", "commit_rev":
"1dd63993f535aa99d4f0ecb3159751b5fcb2089f"}
Message was sent while issue was closed.
Description was changed from ========== [LayoutNG] Place InlineFlowBox from NGFragment tree NGInlineNode::CopyFragmentDataToLayoutBox copies geometry from inline NGFragment tree to RootInlineBox and InlineTextBox, but didn't include InlineFlowBox. This results in InlineFlowBox, generated for nested inline elements, to have (0,0). Their children are placed, but since parent InlineFlowBox is at (0,0), they were clipped. This fix turns several tests to fail. They used to pass because expected images did not render. This fix unhides such tests. BUG=636993, 723117 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng ========== to ========== [LayoutNG] Place InlineFlowBox from NGFragment tree NGInlineNode::CopyFragmentDataToLayoutBox copies geometry from inline NGFragment tree to RootInlineBox and InlineTextBox, but didn't include InlineFlowBox. This results in InlineFlowBox, generated for nested inline elements, to have (0,0). Their children are placed, but since parent InlineFlowBox is at (0,0), they were clipped. This fix turns several tests to fail. They used to pass because expected images did not render. This fix unhides such tests. BUG=636993, 723117 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_layout_ng Review-Url: https://codereview.chromium.org/2887623002 Cr-Commit-Position: refs/heads/master@{#473444} Committed: https://chromium.googlesource.com/chromium/src/+/1dd63993f535aa99d4f0ecb31597... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:140001) as https://chromium.googlesource.com/chromium/src/+/1dd63993f535aa99d4f0ecb31597... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
