Bug 289862 – [Cleanup] Use render and node subclasses instead of RenderObject and Node wherever possible (cont...)

• Component: New Bugs
• Status: RESOLVED FIXED

[alan]

ssia

[alan]

Created attachment 474579 [details]
Patch

[alan]

Created attachment 474582 [details]
Patch

[Tim Nguyen (:ntim)]

Comment on attachment 474582 [details]
Patch

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

> Source/WebCore/dom/Position.cpp:900
> +    auto isHorizontal = renderer.isHorizontalWritingMode();

I think Elika wanted to get rid of `isHorizontalWritingMode()` (so essentially all the usages left are those that need to be audited). I believe `writingMode().isHorizontal()` is preferred now.

> Source/WebCore/editing/Editing.cpp:926
> +    if (!textNode || !downcast<RenderText>(*position.anchorNode()->renderer()).style().preserveNewline())

`!downcast<RenderText>(textNode->renderer())`

also, `renderer()` for `Text` could return `RenderText` directly but that could be a follow up.

[alan]

Created attachment 474630 [details]
Patch

[alan]

Created attachment 474631 [details]
Patch

[alan]

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

[alan]

(In reply to Tim Nguyen (:ntim) from comment #3)
> Comment on attachment 474582 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=474582&action=review
> 
> > Source/WebCore/dom/Position.cpp:900
> > +    auto isHorizontal = renderer.isHorizontalWritingMode();
> 
> I think Elika wanted to get rid of `isHorizontalWritingMode()` (so
> essentially all the usages left are those that need to be audited). I
> believe `writingMode().isHorizontal()` is preferred now.
That's a branchy (slow) path. I am changing it to make it performant but for now that's certainly not the preferred way from pref point of view. 

> 
> > Source/WebCore/editing/Editing.cpp:926
> > +    if (!textNode || !downcast<RenderText>(*position.anchorNode()->renderer()).style().preserveNewline())
> 
> `!downcast<RenderText>(textNode->renderer())`
> 
> also, `renderer()` for `Text` could return `RenderText` directly but that
> could be a follow up.
? It does return RenderText (since 2013).

[alan]

(In reply to zalan from comment #7)
> (In reply to Tim Nguyen (:ntim) from comment #3)
> > Comment on attachment 474582 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=474582&action=review
> > 
> > > Source/WebCore/dom/Position.cpp:900
> > > +    auto isHorizontal = renderer.isHorizontalWritingMode();
> > 
> > I think Elika wanted to get rid of `isHorizontalWritingMode()` (so
> > essentially all the usages left are those that need to be audited). I
> > believe `writingMode().isHorizontal()` is preferred now.
> That's a branchy (slow) path. I am changing it to make it performant but for
> now that's certainly not the preferred way from pref point of view. 
pref -> perf

[EWS]

Committed 292369@main (a0a5ee68b8d5): <https://commits.webkit.org/292369@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/147428099>