RESOLVED FIXED 205207
[LFC][IFC] Fix fast/text/simple-line-with-br.html 
https://bugs.webkit.org/show_bug.cgi?id=205207
Summary [LFC][IFC] Fix fast/text/simple-line-with-br.html
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
DetailsFormatted DiffDiff
Patch (9.83 KB, patch)
2019-12-13 19:00 PST, zalan 		no flags
DetailsFormatted DiffDiff
Patch (9.93 KB, patch)
2019-12-14 04:28 PST, zalan 		no flags
DetailsFormatted DiffDiff
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.