Bug 145500

Summary:

BR styled clear is ignored while partially relayouting.

Product:

WebKit

Reporter:

ChangSeok Oh <changseok>

Component:

Layout and Rendering

Assignee:

ChangSeok Oh <changseok>

Status:

NEW

Severity:

Normal

CC:

ahmad.saleem792, beidson, commit-queue, darin, dbates, esprehn+autocc, glenn, hyatt, kling, koivisto, kondapallykalyan, mmaxfield, rniwa, simon.fraser, zalan

Priority:

Keywords:

BlinkMergeCandidate

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	beidson: review-

ChangSeok Oh

Reported 2015-05-31 09:07:35 PDT

A block containing a br tag with clear style collapses a layout under some cases. I fixed this bug for Blink, and now I'd like to bring the patch to WebKit as well. https://codereview.chromium.org/1102953002

Attachments
Patch (8.37 KB, patch) 2015-05-31 09:14 PDT, ChangSeok Oh	no flags	Details Formatted Diff Diff
Patch (8.35 KB, patch) 2015-06-01 11:28 PDT, ChangSeok Oh	beidson: review-	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

ChangSeok Oh

Comment 1 2015-05-31 09:14:33 PDT

Created attachment 253980 [details] Patch

Alexey Proskuryakov

Comment 2 2015-06-01 00:07:56 PDT

Comment on attachment 253980 [details] Patch I wonder if this is related at all to bug 126169 and bug 140061 (it doesn't sound too close, but there seems to be some potential similarity).

ChangSeok Oh

Comment 3 2015-06-01 11:25:04 PDT

(In reply to comment #2) > Comment on attachment 253980 [details] > Patch > > I wonder if this is related at all to bug 126169 and bug 140061 (it doesn't > sound too close, but there seems to be some potential similarity). No. I don't think so, The issue dealt with my fix happens on a block containing floating objects and a br with 'clear' style at the same time. It seems not relevant with the tests you fixed.

ChangSeok Oh

Comment 4 2015-06-01 11:28:22 PDT

Created attachment 254009 [details] Patch

ChangSeok Oh

Comment 5 2015-06-10 20:55:28 PDT

Would anyone review this? I've cc'ed everyone who can likely review layout code I think.

Darin Adler

Comment 6 2015-06-11 10:12:08 PDT

I think Hyatt is the best reviewer for this. Are you available to review, Dave?

Simon Fraser (smfr)

Comment 7 2015-08-07 13:20:22 PDT

Dave is out for about 3 weeks.

Brady Eidson

Comment 8 2017-04-24 19:07:37 PDT

Comment on attachment 254009 [details] Patch This patch has been pending review since 2015 with no recent activity. It seems unlikely that it would even still apply to trunk in its current form. Clearing from the review queue. Feel free to update and resubmit if the patch is still relevant.

Ahmad Saleem

Comment 9 2022-10-31 17:16:47 PDT

I took the test cases from Chrome patch and it seems they render same in Safari 16.1 as Chrome Canary 109 and Firefox Nightly 108. Test Cases - https://jsfiddle.net/msj2tcbp/show & https://jsfiddle.net/co2ehrnp/show Do we need to merge this patch? If yes, here are the places in Webkit source: https://github.com/WebKit/WebKit/blob/a6ae8781009892c1023a2a28512fefbe724425c8/Source/WebCore/rendering/line/LineLayoutState.h#L108 and https://github.com/WebKit/WebKit/blob/89d57a6f366a8abe5d012e38f160f81388b46429/Source/WebCore/rendering/LegacyLineLayout.cpp#L2122

alan

Comment 10 2022-11-02 12:44:44 PDT

This has progressed with IFC but still broken with legacy line layout. We should probably merge it in.

Ahmad Saleem

Comment 11 2022-11-02 12:46:24 PDT

(In reply to zalan from comment #10) > This has progressed with IFC but still broken with legacy line layout. We > should probably merge it in. Cool, I'll try it once I land current on-going patches. :-)

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