Bug 213052 – [Line clamp] Do not apply the special anchor handling when the anchor content is visible after clamping

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

[alan]

<rdar://problem/59739131>

[alan]

Created attachment 401592 [details]
Patch

[Darin Adler]

Comment on attachment 401592 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Line clamping tries to preserve the anchor text if it is at the bottom of the paragraph to support cases like "... Read mode", where the "read more" is an actual link.

mode/more?

[alan]

Created attachment 401615 [details]
Patch

[EWS]

Committed r262892: <https://trac.webkit.org/changeset/262892>

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

[David Kilzer (:ddkilzer)]

Comment on attachment 401615 [details]
Patch

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

> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:1004
> -        if (anchorBox && anchorBox->renderer().style().isLink())
> +        auto& anchorRenderer = anchorBox->renderer();
> +        auto& lastVisibleRenderer = lastVisibleLine->firstChild()->renderer();
> +        if (anchorBox && anchorBox->renderer().style().isLink() && &lastVisibleRenderer != &anchorRenderer)

Is Line 1002 (auto& anchorRenderer = anchorBox->renderer();) susceptible to a nullptr crash since you're no longer checking `anchorBox` before dereferencing it?

[alan]

(In reply to David Kilzer (:ddkilzer) from comment #5)
> Comment on attachment 401615 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=401615&action=review
> 
> > Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:1004
> > -        if (anchorBox && anchorBox->renderer().style().isLink())
> > +        auto& anchorRenderer = anchorBox->renderer();
> > +        auto& lastVisibleRenderer = lastVisibleLine->firstChild()->renderer();
> > +        if (anchorBox && anchorBox->renderer().style().isLink() && &lastVisibleRenderer != &anchorRenderer)
> 
> Is Line 1002 (auto& anchorRenderer = anchorBox->renderer();) susceptible to
> a nullptr crash since you're no longer checking `anchorBox` before
> dereferencing it?
It should never really happen but I'll add a check. Thanks.

[alan]

(In reply to zalan from comment #6)
> (In reply to David Kilzer (:ddkilzer) from comment #5)
> > Comment on attachment 401615 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=401615&action=review
> > 
> > > Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:1004
> > > -        if (anchorBox && anchorBox->renderer().style().isLink())
> > > +        auto& anchorRenderer = anchorBox->renderer();
> > > +        auto& lastVisibleRenderer = lastVisibleLine->firstChild()->renderer();
> > > +        if (anchorBox && anchorBox->renderer().style().isLink() && &lastVisibleRenderer != &anchorRenderer)
> > 
> > Is Line 1002 (auto& anchorRenderer = anchorBox->renderer();) susceptible to
> > a nullptr crash since you're no longer checking `anchorBox` before
> > dereferencing it?
> It should never really happen but I'll add a check. Thanks.
The InlineBox interface should probably be changed to return references.