Bug 205207

Summary:

[LFC][IFC] Fix fast/text/simple-line-with-br.html

Product:

WebKit

Reporter:

zalan <zalan>

Component:

Layout and Rendering

Assignee:

zalan <zalan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, koivisto, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

zalan

Reported 2019-12-13 08:53:56 PST

ssia

Attachments
Patch (8.33 KB, patch) 2019-12-13 09:00 PST, zalan	no flags	Details Formatted Diff Diff
Patch (9.83 KB, patch) 2019-12-13 19:00 PST, zalan	no flags	Details Formatted Diff Diff
Patch (9.93 KB, patch) 2019-12-14 04:28 PST, zalan	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2019-12-13 08:54:21 PST

<rdar://problem/57913504>

zalan

Comment 2 2019-12-13 09:00:46 PST

Created attachment 385608 [details] Patch

zalan

Comment 3 2019-12-13 09:10:44 PST

Committed r253480: <https://trac.webkit.org/changeset/253480>

zalan

Comment 4 2019-12-13 19:00:07 PST

Reopening to attach new patch.

zalan

Comment 5 2019-12-13 19:00:08 PST

Created attachment 385668 [details] Patch

Antti Koivisto

Comment 6 2019-12-14 02:36:27 PST

Comment on attachment 385668 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385668&action=review > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:419 > + auto invert = false; Invert what? > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:427 > + if (!(invert ^ isFirstLine)) invert == isFirstLine is easier to understand than bitwise math. Or something like auto shouldIndent = invert != isFirstLine; if (!shouldIndent) return {}; > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:431 > + return geometry().computedTextIndent(root, usedHorizontalValues.constraints).valueOr(0); > + }; > + lineLogicalLeft += computedTextIndent().valueOr(0); You should decide which level (lambda or caller) converts Optionals to 0s.

zalan

Comment 7 2019-12-14 04:28:05 PST

Created attachment 385685 [details] Patch

zalan

Comment 8 2019-12-14 04:45:11 PST

(In reply to Antti Koivisto from comment #6) > Comment on attachment 385668 [details] > Patch lol I used the wrong bug. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=385668&action=review > > > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:419 > > + auto invert = false; > > Invert what? > > > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:427 > > + if (!(invert ^ isFirstLine)) > > invert == isFirstLine is easier to understand than bitwise math. Or > something like > > auto shouldIndent = invert != isFirstLine; > if (!shouldIndent) > return {}; make sense. > > > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:431 > > + return geometry().computedTextIndent(root, usedHorizontalValues.constraints).valueOr(0); > > + }; > > + lineLogicalLeft += computedTextIndent().valueOr(0); > > You should decide which level (lambda or caller) converts Optionals to 0s. copy pasta :(

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