Bug 194479 - [LFC][IFC] Add intrinsic width support for inline-block boxes
Summary: [LFC][IFC] Add intrinsic width support for inline-block boxes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-02-09 22:52 PST by zalan
Modified: 2019-02-10 10:04 PST (History)
5 users (show)

See Also:


Attachments
Patch (6.44 KB, patch)
2019-02-09 22:59 PST, zalan
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2019-02-09 22:52:17 PST
ssia
Comment 1 zalan 2019-02-09 22:59:34 PST
Created attachment 361625 [details]
Patch
Comment 2 Antti Koivisto 2019-02-09 23:22:08 PST
Comment on attachment 361625 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=361625&action=review

> Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:160
> +    auto usedValues = UsedHorizontalValues { { }, { }, { } };

Nice. Maybe give the fields = { } so you could just say UsedHorizontalValues { } here?

> Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:163
> +    layoutState().createFormattingContext(layoutBox)->instrinsicWidthConstraints();

It looks weird that this calls instrinsicWidthConstraints() but doesn't do anything with the return value. It would be nice if functions with side effect wouldn't sound like accessors.
Comment 3 zalan 2019-02-10 09:48:29 PST
(In reply to Antti Koivisto from comment #2)
> Comment on attachment 361625 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=361625&action=review
> 
> > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:160
> > +    auto usedValues = UsedHorizontalValues { { }, { }, { } };
> 
> Nice. Maybe give the fields = { } so you could just say UsedHorizontalValues
> { } here?
> 
> > Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp:163
> > +    layoutState().createFormattingContext(layoutBox)->instrinsicWidthConstraints();
> 
> It looks weird that this calls instrinsicWidthConstraints() but doesn't do
> anything with the return value. It would be nice if functions with side
> effect wouldn't sound like accessors.
Yeah, I need to fix this eventually. They are either accessor functions and the caller saves the return value to the state or rename them to computeInstrinsicWidthConstraints(). I'll get back to it once I am done with floats.
Comment 4 zalan 2019-02-10 10:03:59 PST
Committed r241250: <https://trac.webkit.org/changeset/241250>
Comment 5 Radar WebKit Bug Importer 2019-02-10 10:04:27 PST
<rdar://problem/47949584>