Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(63)

Issue 1178893003: Fix regression on positioned movement layout on a line from r195813 (Closed)

Created:
4 years, 10 months ago by rhogan
Modified:
4 years, 10 months ago
CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Fix regression on positioned movement layout on a line from r195813 In https://codereview.chromium.org/1128713004 I inadvertently removed the layout layoutPositionedObjects() was setting on objects that had a static inline position and were inline objects made into blocks by virtue of being positioned. I did this by wrong-headedly altering the isOriginalDisplayInlineType() to isDisplayInlineType() when refactoring. This mistake was motivated by misunderstanding the root of failures I saw locally when using isOriginalDisplayInlineType() in my refactored code. This CL is what I should have done the first time: prior to r195813 we always set a layout on positioned objects with a static block position regardless of whether they were part of an inline or block flow. I incorrectly changed that to only happen in inline flows when the object was itself block flow. Restore the needed layouts so that our only difference versus the state of affairs prior to r195813 is we don't set unnecessary layouts on block flow elements in block flow layout. BUG=498290 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196977

Patch Set 1 #

Patch Set 2 : Updated #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -1 line) Patch
A LayoutTests/fast/block/positioning/positioned-layout-in-line.html View 1 chunk +25 lines, -0 lines 0 comments Download
A LayoutTests/fast/block/positioning/positioned-layout-in-line-expected.html View 1 chunk +18 lines, -0 lines 0 comments Download
M Source/core/layout/LayoutBlock.cpp View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (2 generated)
rhogan
4 years, 10 months ago (2015-06-11 18:49:37 UTC) #2
leviw_travelin_and_unemployed
lgtm Thanks for the helpful description!
4 years, 10 months ago (2015-06-11 21:08:19 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1178893003/20001
4 years, 10 months ago (2015-06-11 21:08:32 UTC) #5
commit-bot: I haz the power
4 years, 10 months ago (2015-06-11 22:28:40 UTC) #6
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196977

Powered by Google App Engine
This is Rietveld 408576698