Bug 278030 – [text-box-edge/line-fit-edge] Add support for line-fit-edge and update text-box-edge

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[zalan]

see https://drafts.csswg.org/css-inline-3/#text-edges

[zalan]

Created attachment 472141 [details]
Patch

[zalan]

tests needs to be updated before landing this.

[Antti Koivisto]

Comment on attachment 472141 [details]
Patch

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

> Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h:171
> -        TextBoxEdge textBoxEdge;
> +        TextEdge textBoxEdge;
> +        TextEdge lineFitEdge;

Do we actually need both values here or could they just be reduces to a single value when constructing the Style?

[zalan]

(In reply to Antti Koivisto from comment #3)
> Comment on attachment 472141 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=472141&action=review
> 
> > Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h:171
> > -        TextBoxEdge textBoxEdge;
> > +        TextEdge textBoxEdge;
> > +        TextEdge lineFitEdge;
> 
> Do we actually need both values here or could they just be reduces to a
> single value when constructing the Style?
I think you are right. We only need one (root vs. non-root inline box)

[zalan]

Created attachment 472142 [details]
Patch

[fantasai]

Comment on attachment 472141 [details]
Patch

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

> Source/WebCore/layout/formattingContexts/inline/InlineLevelBoxInlines.h:51
> +        m_style.textBoxEdge = m_style.lineFitEdge;

Not sure what's happening here, but the text-box-edge in question needs to be the one of the block container requesting the trim (which may be an ancestor).

[zalan]

Created attachment 472146 [details]
[fast-cq]Patch

[Antti Koivisto]

Comment on attachment 472146 [details]
[fast-cq]Patch

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

> Source/WebCore/rendering/style/RenderStyleConstants.h:722
> +    Auto,
>      Leading,

maybe add a comment that these values only show up in a certain properties

[EWS]

Committed 282228@main (5163a893fe8b): <https://commits.webkit.org/282228@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 472146 [details].

[Radar WebKit Bug Importer]

<rdar://problem/133834296>

[zalan]

(In reply to Antti Koivisto from comment #8)
> Comment on attachment 472146 [details]
> [fast-cq]Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=472146&action=review
> 
> > Source/WebCore/rendering/style/RenderStyleConstants.h:722
> > +    Auto,
> >      Leading,
> 
> maybe add a comment that these values only show up in a certain properties
oops, gonna address this as post-landing. :(