New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 780927 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Incorrect list bullets position after node with style=float:left is removed

Reported by robo.bur...@gmail.com, Nov 2 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.75 Safari/537.36

Example URL:
https://codepen.io/anon/pen/xPZgPb

Steps to reproduce the problem:
1. Navigate to https://codepen.io/anon/pen/xPZgPb
2. Click Break Chrome button

What is the expected behavior?
DIV should be removed. Lists with bullets and numbers should be moved to the appeared free space on page.

What went wrong?
When a float div node is removed, the lists after the div are moved to the left of the screen, but list numbers and bullets are separated from the list items and stay on initial position

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? Yes Chrome 61

Does this work in other browsers? Yes

Chrome version: 62.0.3202.75  Channel: stable
OS Version: 10.0
Flash Version:
 
Break.html
363 bytes View Download
Labels: Needs-Bisect Needs-Triage-M62
Cc: sc00335...@techmahindra.com
Components: Blink>CSS
Labels: -Pri-2 -Type-Compat -Needs-Bisect hasbisect-per-revision Triaged-ET M-64 OS-Linux OS-Mac Pri-1 Type-Bug-Regression
Owner: robho...@gmail.com
Status: Assigned (was: Unconfirmed)
Able to reproduce this issue on reported version 62.0.3202.75 and on latest canary 64.0.3256.0 using Windows10, Ubuntu 14.04 and Mac 10.12.6 with steps mentioned in comment#0.

Manual Bisect Info:
==================
Good Build:57.0.2948.0 
Bad Build:57.0.2949.0

You are probably looking for a change made after 437797 (known good), but no later than 437798 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/73ac3ff0ecf46d891ec19cad2261472c9a4292ea..c896e79e5ba348d7ed87438cd3a19d0176f3036d

Review-Url: https://codereview.chromium.org/2553793003

Suspecting same from changelog.

@robhogan: Please confirm the bug and help in re-assigning if it is not related your change.

Thanks!
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a22edaa3ae7ef77e5d2499062bbccf24f1f5f1c

commit 7a22edaa3ae7ef77e5d2499062bbccf24f1f5f1c
Author: Robert Hogan <robhogan@gmail.com>
Date: Fri Nov 03 20:06:32 2017

List markers should always get a layout when the list item does.

A follow-on to https://chromium-review.googlesource.com/602352.

Turns out it is safer to always update the list marker position after
laying out a list item.

Bug:  780927 
Change-Id: I09ef8e38bcb65a33593d8b6198f29dbd9dce26c7
Reviewed-on: https://chromium-review.googlesource.com/753362
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513886}
[add] https://crrev.com/7a22edaa3ae7ef77e5d2499062bbccf24f1f5f1c/third_party/WebKit/LayoutTests/fast/lists/list-marker-avoid-float-4-expected.html
[add] https://crrev.com/7a22edaa3ae7ef77e5d2499062bbccf24f1f5f1c/third_party/WebKit/LayoutTests/fast/lists/list-marker-avoid-float-4.html
[modify] https://crrev.com/7a22edaa3ae7ef77e5d2499062bbccf24f1f5f1c/third_party/WebKit/Source/core/layout/LayoutBlock.cpp

Comment 4 by meade@chromium.org, Nov 7 2017

Cc: meade@chromium.org
Labels: Update-Quarterly
Hey Rob, is this fixed with your change?

Comment 5 by robho...@gmail.com, Nov 7 2017

Yes, it should be fixed with this change.

Comment 6 by meade@chromium.org, Nov 7 2017

Status: Fixed (was: Assigned)

Comment 7 by robho...@gmail.com, Nov 22 2017

Labels: Merge-Request-63
Project Member

Comment 8 by sheriffbot@chromium.org, Nov 22 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: We are only 12 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 9 by gov...@chromium.org, Nov 23 2017

Labels: -Merge-Review-63 Merge-Rejected-63
Per comment #2, this has been regressed since M57 and we're very close to M63 stable promotion so taking only critical merges in at this point. Hence, rejecting merge to M63. Please let me know if there is any concern here. 
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 28 2017

Labels: merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a40486e605f7606e7a57b849d9789c6dabdc2e4

commit 2a40486e605f7606e7a57b849d9789c6dabdc2e4
Author: Christian Biesinger <cbiesinger@chromium.org>
Date: Tue Nov 28 18:08:24 2017

List markers should always get a layout when the list item does.

A follow-on to https://chromium-review.googlesource.com/602352.

Turns out it is safer to always update the list marker position after
laying out a list item.

TBR=robhogan@gmail.com

(cherry picked from commit 7a22edaa3ae7ef77e5d2499062bbccf24f1f5f1c)

Bug:  780927 
Change-Id: I09ef8e38bcb65a33593d8b6198f29dbd9dce26c7
Reviewed-on: https://chromium-review.googlesource.com/753362
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#513886}
Reviewed-on: https://chromium-review.googlesource.com/794035
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#583}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[add] https://crrev.com/2a40486e605f7606e7a57b849d9789c6dabdc2e4/third_party/WebKit/LayoutTests/fast/lists/list-marker-avoid-float-4-expected.html
[add] https://crrev.com/2a40486e605f7606e7a57b849d9789c6dabdc2e4/third_party/WebKit/LayoutTests/fast/lists/list-marker-avoid-float-4.html
[modify] https://crrev.com/2a40486e605f7606e7a57b849d9789c6dabdc2e4/third_party/WebKit/Source/core/layout/LayoutBlock.cpp

Merge approval happened in 787639
Project Member

Comment 12 by bugdroid1@chromium.org, May 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/362aefe984ef8c2b8f7f2a03b5c747365950e3e4

commit 362aefe984ef8c2b8f7f2a03b5c747365950e3e4
Author: Aleks Totic <atotic@chromium.org>
Date: Tue May 01 06:52:39 2018

Revert "List markers should always get a layout when the list item does."

This reverts commit 7a22edaa3ae7ef77e5d2499062bbccf24f1f5f1c.

Reason for revert: Multiple regressions like 834628, 835371.

Original change's description:
> List markers should always get a layout when the list item does.
>
> A follow-on to https://chromium-review.googlesource.com/602352.
>
> Turns out it is safer to always update the list marker position after
> laying out a list item.
>
> Bug:  780927 
> Change-Id: I09ef8e38bcb65a33593d8b6198f29dbd9dce26c7
> Reviewed-on: https://chromium-review.googlesource.com/753362
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Commit-Queue: Emil A Eklund <eae@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#513886}

TBR=robhogan@gmail.com,eae@chromium.org

Change-Id: I83c216d4b364360c3d593cc91eac95327fad3c33
Reviewed-on: https://chromium-review.googlesource.com/1036856
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Abhishek Arya <inferno@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Commit-Queue: Abhishek Arya <inferno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555010}
[modify] https://crrev.com/362aefe984ef8c2b8f7f2a03b5c747365950e3e4/third_party/blink/renderer/core/layout/layout_block.cc

Project Member

Comment 13 by bugdroid1@chromium.org, May 1 2018

Labels: merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc36611260ea8e98f26e9384b0471531bd4d331e

commit dc36611260ea8e98f26e9384b0471531bd4d331e
Author: Emil A Eklund <eae@chromium.org>
Date: Tue May 01 17:57:20 2018

Revert "List markers should always get a layout when the list item does."

This reverts commit 7a22edaa3ae7ef77e5d2499062bbccf24f1f5f1c.

Reason for revert: Multiple regressions like 834628, 835371.

Original change's description:
> List markers should always get a layout when the list item does.
>
> A follow-on to https://chromium-review.googlesource.com/602352.
>
> Turns out it is safer to always update the list marker position after
> laying out a list item.
>
> Bug:  780927 
> Change-Id: I09ef8e38bcb65a33593d8b6198f29dbd9dce26c7
> Reviewed-on: https://chromium-review.googlesource.com/753362
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Commit-Queue: Emil A Eklund <eae@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#513886}

TBR=atotic@chromium.org, eae@chromium.org, robhogan@gmail.com

(cherry picked from commit 362aefe984ef8c2b8f7f2a03b5c747365950e3e4)

Change-Id: I83c216d4b364360c3d593cc91eac95327fad3c33
Reviewed-on: https://chromium-review.googlesource.com/1036856
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Abhishek Arya <inferno@chromium.org>
Commit-Queue: Aleks Totic <atotic@chromium.org>
Commit-Queue: Abhishek Arya <inferno@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555010}
Reviewed-on: https://chromium-review.googlesource.com/1037763
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#412}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/dc36611260ea8e98f26e9384b0471531bd4d331e/third_party/blink/renderer/core/layout/layout_block.cc

Labels: Needs-Feedback
Able to reproduce this issue on reported version hence verifying the fix on latest canary 68.0.3417.0 and latest beta 67.0.3396.30

Interestingly even with the CL reverted this seems to be working fine on the latest M-67 and M-68. Attaching screenshots for reference.

@atotic/@eae: Please help in verifying the fix.

Thanks!

780927-68.0.3417.0.PNG
211 KB View Download
780927-67.0.3396.30PNG.PNG
201 KB View Download
Cc: pbomm...@chromium.org gov...@chromium.org e...@chromium.org
Labels: ReleaseBlock-Stable M-67
Status: Available (was: Fixed)
Based on offline chat with Emil we are going ahead with M67 beta refresh.

But I am marking the bug as available and tagging with stable blocker for now.
*** Bulk Edit ***
M67 Stable promotion is coming soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. 

If fix is already merged to M67 and nothing else is pending, pls mark the bug as fixed. Thank you.
*** Bulk Edit ***
M67 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. 

If fix is already merged to M67 and nothing else is pending, pls mark the bug as fixed. Thank you.
Owner: e...@chromium.org
Status: Assigned (was: Available)

Comment 19 by e...@chromium.org, May 8 2018

Labels: Merge-Request-67
Project Member

Comment 20 by sheriffbot@chromium.org, May 8 2018

Labels: -Merge-Request-67 Merge-Review-67
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 21 by e...@chromium.org, May 8 2018

Labels: -ReleaseBlock-Stable -Merge-Review-67
Status: Fixed (was: Assigned)
Err, wrong bug, my bad. This should be closed.

Sign in to add a comment