Bug 237951 – [selection] Change form's selection attribute to unsigned long

• Component: Forms
• Status: RESOLVED FIXED

We should use unsigned long for the type of selectionStart and selectionEnd.

[Radar WebKit Bug Importer]

<rdar://problem/90364891>

Created attachment 454832 [details]
Patch

Created attachment 454954 [details]
Patch

Created attachment 454955 [details]
Patch

Created attachment 454957 [details]
Patch

Created attachment 454960 [details]
Patch

Created attachment 454963 [details]
Patch

Created attachment 454968 [details]
Patch

Any chance for a review? Thank you.

[Chris Dumez]

Comment on attachment 454968 [details]
Patch

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

r=me

> Source/WebCore/html/HTMLTextFormControlElement.h:183
> +    bool m_hasCachedSelection { false };

I think personally I would have gone with a `std::optional<std::pair<unsigned, unsigned>>` but this is fine too.

[EWS]

Committed r291609 (248702@main): <https://commits.webkit.org/248702@main>

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

Reopening to attach new patch.

Created attachment 455500 [details]
Patch

[Darin Adler]

Comment on attachment 455500 [details]
Patch

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

Looks pretty good. One question.

> Source/WebCore/html/HTMLTextFormControlElement.cpp:327
>          if (!isTextField())
> -            return;
> +            return false;

Is it OK here that we already did the cacheSelection operation? The old code would not have done that. What tests cover this change in behavior?

[Darin Adler]

Comment on attachment 455500 [details]
Patch

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

> Source/WebCore/ChangeLog:986
> +        [selection] Fire Select event when selection extent or direction changes
> +        https://bugs.webkit.org/show_bug.cgi?id=238142

Looks like this new patch to review got attached to the wrong bug.

(In reply to Darin Adler from comment #15)
> Comment on attachment 455500 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=455500&action=review
> 
> > Source/WebCore/ChangeLog:986
> > +        [selection] Fire Select event when selection extent or direction changes
> > +        https://bugs.webkit.org/show_bug.cgi?id=238142
> 
> Looks like this new patch to review got attached to the wrong bug.

Ah...I was careless. Just changed the status of the patch to obsolete.

(In reply to Darin Adler from comment #14)
> Comment on attachment 455500 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=455500&action=review
> 
> Looks pretty good. One question.
> 
> > Source/WebCore/html/HTMLTextFormControlElement.cpp:327
> >          if (!isTextField())
> > -            return;
> > +            return false;
> 
> Is it OK here that we already did the cacheSelection operation? The old code
> would not have done that. What tests cover this change in behavior?

Let me check on this.