Bug 224820 – Rename WebCore::SelectionRect to WebCore::SelectionGeometry

• Component: HTML Editing
• Status: RESOLVED FIXED

[Wenson Hsieh]

.

[Wenson Hsieh]

Created attachment 426567 [details]
For EWS

[Darin Adler]

Comment on attachment 426567 [details]
For EWS

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

> Source/WebCore/platform/ios/SelectionGeometry.cpp:88
> +SelectionGeometry::SelectionGeometry()
> +    : m_direction(TextDirection::LTR)
> +    , m_minX(0)
> +    , m_maxX(0)
> +    , m_maxY(0)
> +    , m_lineNumber(0)
> +    , m_isLineBreak(false)
> +    , m_isFirstOnLine(false)
> +    , m_isLastOnLine(false)
> +    , m_containsStart(false)
> +    , m_containsEnd(false)
> +    , m_isHorizontal(true)
> +    , m_isInFixedPosition(false)
> +    , m_isRubyText(false)
> +    , m_pageNumber(0)
> +{
> +}

Apparently this was just moved code, but I suggest we initialize all of these in the class definition instead, and just have the default constructor be "= default" or something like that.

Relying on this should cut down the length of at least one of the other constructors, too, and make it harder to accidentally leave anything uninitialized.

> Source/WebCore/platform/ios/SelectionGeometry.h:98
> +    void setLogicalLeft(int left)
> +    {
> +        if (m_isHorizontal)
> +            m_rect.setX(left);
> +        else
> +            m_rect.setY(left);
> +    }
> +
> +    void setLogicalWidth(int width)
> +    {
> +        if (m_isHorizontal)
> +            m_rect.setWidth(width);
> +        else
> +            m_rect.setHeight(width);
> +    }
> +
> +    void setLogicalTop(int top)
> +    {
> +        if (m_isHorizontal)
> +            m_rect.setY(top);
> +        else
> +            m_rect.setX(top);
> +    }
> +
> +    void setLogicalHeight(int height)
> +    {
> +        if (m_isHorizontal)
> +            m_rect.setHeight(height);
> +        else
> +            m_rect.setWidth(height);
> +    }

Also not new: I suggest moving these inline function bodies out of the class definition, to make it easier to read it and get an overview.

[Wenson Hsieh]

Comment on attachment 426567 [details]
For EWS

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

>> Source/WebCore/platform/ios/SelectionGeometry.cpp:88
>> +}
> 
> Apparently this was just moved code, but I suggest we initialize all of these in the class definition instead, and just have the default constructor be "= default" or something like that.
> 
> Relying on this should cut down the length of at least one of the other constructors, too, and make it harder to accidentally leave anything uninitialized.

Ah, so my next patch coming up does exactly this; I'll move that change into part of this patch instead.

>> Source/WebCore/platform/ios/SelectionGeometry.h:98
>> +    }
> 
> Also not new: I suggest moving these inline function bodies out of the class definition, to make it easier to read it and get an overview.

Sounds good! Will move these implementations into the cpp.

[Wenson Hsieh]

Created attachment 426583 [details]
For landing

[EWS]

Committed r276331 (236811@main): <https://commits.webkit.org/236811@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/77062455>