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

Issue 68013006: Fix render tree construction when inlines are inserted in reverse after a block in a continuation (Closed)

Created:
7 years, 1 month ago by leviw_travelin_and_unemployed
Modified:
7 years, 1 month ago
Reviewers:
esprehn, eae, ojan
CC:
blink-reviews, bemjb+rendering_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, zoltan1
Visibility:
Public.

Description

Fix render tree construction when inlines are inserted in reverse after a block in a continuation When inserting inlines after a block inside of an inline continuation, if we insert inline content in reverse DOM order, we can (depending on when we attach) end up appending the renderers in the wrong location, leading to visual aberrations. Since Blink r158839, we always attach in reverse DOM order, which makes this bug easier to run into. The fix is in RenderInline, where we should always attach behind our beforeChild if we're inserting an inline and beforeChild is an inline, regardless of other continuation state. Big thanks to Elliott for the test case! BUG=318805 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162160

Patch Set 1 #

Total comments: 1

Patch Set 2 : Updating with eae's comments #

Patch Set 3 : Adding NeedsRebaseline for table test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -8 lines) Patch
M LayoutTests/TestExpectations View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/inline/continuation-inlines-inserted-in-reverse-after-block.html View 1 1 chunk +33 lines, -0 lines 0 comments Download
A LayoutTests/fast/inline/continuation-inlines-inserted-in-reverse-after-block-expected.txt View 1 chunk +13 lines, -0 lines 0 comments Download
M LayoutTests/platform/win/tables/mozilla/bugs/bug113235-3-expected.txt View 2 chunks +46 lines, -4 lines 0 comments Download
M Source/core/rendering/RenderInline.cpp View 1 1 chunk +3 lines, -4 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
leviw_travelin_and_unemployed
7 years, 1 month ago (2013-11-16 01:36:44 UTC) #1
eae
LGTM https://codereview.chromium.org/68013006/diff/1/LayoutTests/fast/inline/continuation-inlines-inserted-in-reverse-after-block.html File LayoutTests/fast/inline/continuation-inlines-inserted-in-reverse-after-block.html (right): https://codereview.chromium.org/68013006/diff/1/LayoutTests/fast/inline/continuation-inlines-inserted-in-reverse-after-block.html#newcode10 LayoutTests/fast/inline/continuation-inlines-inserted-in-reverse-after-block.html:10: var last = null; var last; will do, ...
7 years, 1 month ago (2013-11-16 01:45:37 UTC) #2
esprehn
lgtm, awesome
7 years, 1 month ago (2013-11-16 03:19:58 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/leviw@chromium.org/68013006/60001
7 years, 1 month ago (2013-11-16 22:00:16 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/leviw@chromium.org/68013006/170001
7 years, 1 month ago (2013-11-16 23:20:59 UTC) #5
commit-bot: I haz the power
7 years, 1 month ago (2013-11-17 01:15:38 UTC) #6
Message was sent while issue was closed.
Change committed as 162160

Powered by Google App Engine
This is Rietveld 408576698