[webkit-reviews] review granted: [Bug 205245] [LFC][IFC] Fix imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-word-006.html : [Attachment 385708] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Dec 15 00:03:24 PST 2019
Antti Koivisto <koivisto at iki.fi> has granted zalan <zalan at apple.com>'s request
for review:
Bug 205245: [LFC][IFC] Fix
imported/w3c/web-platform-tests/css/css-text/overflow-wrap/overflow-wrap-break-
word-006.html
https://bugs.webkit.org/show_bug.cgi?id=205245
Attachment 385708: Patch
https://bugs.webkit.org/attachment.cgi?id=385708&action=review
--- Comment #3 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 385708
--> https://bugs.webkit.org/attachment.cgi?id=385708
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.
More information about the webkit-reviews
mailing list