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-

Description ChangSeok Oh 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

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

Created attachment 253980 [details]
Patch

Comment 2 Alexey Proskuryakov 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).

Comment 3 ChangSeok Oh 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.

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

Created attachment 254009 [details]
Patch

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

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

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

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

Comment 7 Simon Fraser (smfr) 2015-08-07 13:20:22 PDT

Dave is out for about 3 weeks.

Comment 8 Brady Eidson 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.

Comment 9 Ahmad Saleem 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

Comment 10 zalan 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.

Comment 11 Ahmad Saleem 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. :-)