166528 – A floating element within <li> overlaps with the marker

RESOLVED FIXED 166528

A floating element within <li> overlaps with the marker

https://bugs.webkit.org/show_bug.cgi?id=166528

Summary A floating element within <li> overlaps with the marker

Carlos Alberto Lopez Perez

Reported 2016-12-28 04:58:19 PST

Example URL: data:text/html,<!doctype html><ol><li><input style="float: left;" type=checkbox> Steps to reproduce the problem: 1. Go to the mentioned URL. What is the expected behavior? A marker and then a checkbox to its right. What went wrong? The marker and the checkbox overlap. This bug has been initially reported to Chromium at http://crbug.com/548616 and fixed in https://chromium.googlesource.com/chromium/src/+/c896e79e5ba348d7ed87438cd3a19d0176f3036d It also affects WebKit.

Attachments
Patch (47.55 KB, patch) 2016-12-28 06:24 PST, Carlos Alberto Lopez Perez	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews117 for mac-elcapitan (1.79 MB, application/zip) 2016-12-28 07:14 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (1.09 MB, application/zip) 2016-12-28 07:24 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews103 for mac-elcapitan (829.69 KB, application/zip) 2016-12-28 07:25 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (9.32 MB, application/zip) 2016-12-28 07:39 PST, Build Bot	no flags	Details
Patch (206.51 KB, patch) 2016-12-28 08:16 PST, Carlos Alberto Lopez Perez	no flags	Details Formatted Diff Diff
Patch (207.01 KB, patch) 2016-12-28 09:16 PST, Carlos Alberto Lopez Perez	no flags	Details Formatted Diff Diff
Patch (12.39 KB, patch) 2017-01-03 06:02 PST, Carlos Alberto Lopez Perez	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Carlos Alberto Lopez Perez

Comment 1 2016-12-28 05:02:31 PST

(In reply to comment #0) > Example URL: > data:text/html,<!doctype html><ol><li><input style="float: left;" > type=checkbox> > I uploaded the initial test case also here: https://people.igalia.com/clopez/wkbug/166528/

Carlos Alberto Lopez Perez

Comment 2 2016-12-28 06:24:07 PST

Created attachment 297803 [details] Patch Initial patch ported to WebKit with WebKitGTK+ results. Not asking for review still. Waiting for the layout tests results from the EWS

Build Bot

Comment 3 2016-12-28 07:14:20 PST

Comment on attachment 297803 [details] Patch Attachment 297803 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2798467 New failing tests: fast/lists/list-marker-before-float-nested.html fast/lists/list-marker-before-float-rtl.html fast/lists/list-marker-before-float.html fast/lists/list-marker-before-float-nested-rtl.html

Build Bot

Comment 4 2016-12-28 07:14:23 PST

Created attachment 297804 [details] Archive of layout-test-results from ews117 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 5 2016-12-28 07:24:43 PST

Comment on attachment 297803 [details] Patch Attachment 297803 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2798506 New failing tests: fast/lists/list-marker-before-float-nested.html fast/lists/list-marker-before-float-rtl.html fast/lists/list-marker-before-float.html fast/lists/list-marker-before-float-nested-rtl.html

Build Bot

Comment 6 2016-12-28 07:24:47 PST

Created attachment 297805 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Build Bot

Comment 7 2016-12-28 07:24:59 PST

Comment on attachment 297803 [details] Patch Attachment 297803 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2798514 New failing tests: fast/lists/list-marker-before-float-nested.html fast/lists/list-marker-before-float-rtl.html fast/lists/list-marker-before-float.html fast/lists/list-marker-before-float-nested-rtl.html

Build Bot

Comment 8 2016-12-28 07:25:03 PST

Created attachment 297806 [details] Archive of layout-test-results from ews103 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 9 2016-12-28 07:39:30 PST

Comment on attachment 297803 [details] Patch Attachment 297803 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2798524 New failing tests: fast/lists/list-marker-before-float-nested.html fast/lists/list-marker-before-float-rtl.html fast/lists/list-marker-before-float.html fast/lists/list-marker-before-float-nested-rtl.html

Build Bot

Comment 10 2016-12-28 07:39:34 PST

Created attachment 297807 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6

Carlos Alberto Lopez Perez

Comment 11 2016-12-28 08:16:21 PST

Created attachment 297809 [details] Patch Second try, adding results from EWS. Still not asking for review

Carlos Alberto Lopez Perez

Comment 12 2016-12-28 09:16:20 PST

Created attachment 297813 [details] Patch All EWS green, asking for review now.

alan

Comment 13 2016-12-28 09:50:35 PST

Comment on attachment 297813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=297813&action=review > Source/WebCore/ChangeLog:17 > + Computing the offset for a list marker after the rest of the objects on the line > + it is on have been laid out, means it will avoid floats it ought not to. > + > + Instead, compute the offset when laying out the marker and cache it for use later. This patch seems to be a workaround to compensate for the fact that we position the list markers way too late -during the overflow computation. (hence compute early and cache for later). > Source/WebCore/rendering/RenderListMarker.cpp:1123 > + , m_lineOffset() Not needed. > Source/WebCore/rendering/RenderListMarker.cpp:1378 > + for (RenderBox* o = parentBox(); o && o != &m_listItem; o = o->parentBox()) for (auto* box = parentBox(); box && box !=...) > Source/WebCore/rendering/RenderListMarker.h:45 > + LayoutUnit lineOffset() const { return m_lineOffset; } lineOffset is way too vague (left, right, top, bottom offset from parent, list item,containing block?). This name only works in the context of this particular patch. > LayoutTests/ChangeLog:35 > + * fast/lists/list-marker-before-float-nested-rtl.html: Added. > + * fast/lists/list-marker-before-float-nested.html: Added. > + * fast/lists/list-marker-before-float-rtl.html: Added. > + * fast/lists/list-marker-before-float.html: Added. > + * platform/gtk/fast/lists/list-marker-before-float-expected.png: Added. > + * platform/gtk/fast/lists/list-marker-before-float-expected.txt: Added. > + * platform/gtk/fast/lists/list-marker-before-float-nested-expected.png: Added. > + * platform/gtk/fast/lists/list-marker-before-float-nested-expected.txt: Added. > + * platform/gtk/fast/lists/list-marker-before-float-nested-rtl-expected.png: Added. > + * platform/gtk/fast/lists/list-marker-before-float-nested-rtl-expected.txt: Added. > + * platform/gtk/fast/lists/list-marker-before-float-rtl-expected.png: Added. > + * platform/gtk/fast/lists/list-marker-before-float-rtl-expected.txt: Added. > + * platform/ios-simulator/fast/lists/list-marker-before-float-expected.png: Added. > + * platform/ios-simulator/fast/lists/list-marker-before-float-expected.txt: Added. > + * platform/ios-simulator/fast/lists/list-marker-before-float-nested-expected.png: Added. > + * platform/ios-simulator/fast/lists/list-marker-before-float-nested-expected.txt: Added. > + * platform/ios-simulator/fast/lists/list-marker-before-float-nested-rtl-expected.png: Added. > + * platform/ios-simulator/fast/lists/list-marker-before-float-nested-rtl-expected.txt: Added. > + * platform/ios-simulator/fast/lists/list-marker-before-float-rtl-expected.png: Added. > + * platform/ios-simulator/fast/lists/list-marker-before-float-rtl-expected.txt: Added. > + * platform/mac/fast/lists/list-marker-before-float-expected.png: Added. > + * platform/mac/fast/lists/list-marker-before-float-expected.txt: Added. > + * platform/mac/fast/lists/list-marker-before-float-nested-expected.png: Added. > + * platform/mac/fast/lists/list-marker-before-float-nested-expected.txt: Added. > + * platform/mac/fast/lists/list-marker-before-float-nested-rtl-expected.png: Added. > + * platform/mac/fast/lists/list-marker-before-float-nested-rtl-expected.txt: Added. > + * platform/mac/fast/lists/list-marker-before-float-rtl-expected.png: Added. > + * platform/mac/fast/lists/list-marker-before-float-rtl-expected.txt: Added. Ref tests should be doable.

Carlos Alberto Lopez Perez

Comment 14 2016-12-29 07:02:35 PST

(In reply to comment #13) > Comment on attachment 297813 [details] [...] > > Source/WebCore/rendering/RenderListMarker.h:45 > > + LayoutUnit lineOffset() const { return m_lineOffset; } > > lineOffset is way too vague (left, right, top, bottom offset from parent, > list item,containing block?). This name only works in the context of this > particular patch. > I don't think is vague on the context of being inside a class named "RenderListMarker". Would any of lineOffsetBeforeFloats, logicalOffsetForLine, lineOffsetForListItem sounds good to you? Otherwise I don't think is right saying that is a left/right/top/bottom offset, because it can be any of them depending for example if the page is LTR/RTL.

Carlos Alberto Lopez Perez

Comment 15 2017-01-03 06:02:14 PST

Created attachment 297932 [details] Patch Updated patch trying reftests

Carlos Alberto Lopez Perez

Comment 16 2017-01-03 07:03:29 PST

Committed r210239: <http://trac.webkit.org/changeset/210239>

Darin Adler

Comment 17 2017-01-03 11:37:47 PST

Hooray for the use of reference tests in this patch!

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware Unspecified

OS Unspecified

Product WebKit

Component Layout and Rendering

Assignee

Carlos Alberto Lopez Perez

Reported

2016-12-28 04:58 PST

Modified

2017-01-03 11:37 PST History

CC List

12 users Show

URL

https://people.igalia.com/clopez/wkbug/166528/

Keywords

Depends on

Blocks