Bug 205245 – [LFC][IFC] Fix imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-006.html

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[zalan]

ssia

[Radar WebKit Bug Importer]

<rdar://problem/57943885>

[zalan]

Created attachment 385708 [details]
Patch

[Antti Koivisto]

Comment on attachment 385708 [details]
Patch

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

> Source/WebCore/layout/inlineformatting/InlineLineBreaker.cpp:109
> +        if (auto partialTrailingContent = wrapTextContent(runs, lineStatus)) {

Here the returned variable is called "content".

> Source/WebCore/layout/inlineformatting/InlineLineBreaker.h:125
> +    struct TextWrappingContext {
> +        unsigned trailingRunIndex { 0 };
> +        bool contentOverflows { false };
> +        Optional<PartialRun> partialTrailingRun;
> +    };
> +    Optional<TextWrappingContext> wrapTextContent(const Content::RunList&, const LineStatus&) const;

Not sure I understand how this type is a "context". 

context
noun
1 the historical context out of which the novel arose: circumstances, conditions, surroundings, factors, state of affairs; situation, environment, milieu, setting, background, backdrop, scene; climate, atmosphere, ambience, mood, feel.

It is also confusing that it is returned by function called "content".

Based on your variable naming above, maybe the type should be "PartialTrailingContent"? Or based on the function naming "WrappedTextContent"? In this case you might want to rename the variables too.

[zalan]

Committed r253533: <https://trac.webkit.org/changeset/253533>

[zalan]

(In reply to zalan from comment #4)
> Committed r253533: <https://trac.webkit.org/changeset/253533>

Oh no, I didn't mean to land it without the renaming :(

[zalan]

(In reply to zalan from comment #5)
> (In reply to zalan from comment #4)
> > Committed r253533: <https://trac.webkit.org/changeset/253533>
> 
> Oh no, I didn't mean to land it without the renaming :(

Addressed in r253534.