Created attachment 453212 [details]
Patch

Comment on attachment 453212 [details]
Patch

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

> Source/WebCore/html/HTMLTextFormControlElement.cpp:283
> +    const int innerTextValueLength = innerTextValue().length();
> +    end = std::min(std::max(end, 0), std::max(innerTextValueLength, 0));
>      start = std::min(std::max(start, 0), end);

The "const" on the first line is not WebKit coding style. Most local variables are constant, not reassigned, and we typically don’t put "const" in front of all of them, although we could. I suggest writing this instead:

    constexpr unsigned maxInt = std::numeric_limits<int>::max();
    int innerTextValueLength = std::min(innerTextValue().length(), maxInt);
    end = std::clamp(end, 0, innerTextValueLength);
    start = std::clamp(start, 0, end);

Comment on attachment 453212 [details]
Patch

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

>> Source/WebCore/html/HTMLTextFormControlElement.cpp:283
>>      start = std::min(std::max(start, 0), end);
> 
> The "const" on the first line is not WebKit coding style. Most local variables are constant, not reassigned, and we typically don’t put "const" in front of all of them, although we could. I suggest writing this instead:
> 
>     constexpr unsigned maxInt = std::numeric_limits<int>::max();
>     int innerTextValueLength = std::min(innerTextValue().length(), maxInt);
>     end = std::clamp(end, 0, innerTextValueLength);
>     start = std::clamp(start, 0, end);

Here’s a better idea:

    end = std::clamp(end, 0, clampTo<int>(innerTextValue().length()));
    start = std::clamp(start, 0, end);

Created attachment 453376 [details]
Patch

Committed r290635 (247908@main): <https://commits.webkit.org/247908@main>

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

<rdar://problem/89607167>