Bug 224820 - Rename WebCore::SelectionRect to WebCore::SelectionGeometry
Summary: Rename WebCore::SelectionRect to WebCore::SelectionGeometry
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-20 10:07 PDT by Wenson Hsieh
Modified: 2021-04-23 01:59 PDT (History)
20 users (show)

See Also:


Attachments
For EWS (120.90 KB, patch)
2021-04-20 10:32 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
For landing (120.43 KB, patch)
2021-04-20 13:02 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2021-04-20 10:07:18 PDT
.
Comment 1 Wenson Hsieh 2021-04-20 10:32:37 PDT
Created attachment 426567 [details]
For EWS
Comment 2 Darin Adler 2021-04-20 11:35:53 PDT
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.
Comment 3 Wenson Hsieh 2021-04-20 12:14:53 PDT
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.
Comment 4 Wenson Hsieh 2021-04-20 13:02:03 PDT
Created attachment 426583 [details]
For landing
Comment 5 EWS 2021-04-20 17:04:03 PDT
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].
Comment 6 Radar WebKit Bug Importer 2021-04-23 01:59:41 PDT
<rdar://problem/77062455>