Bug 237361

Summary: [InputElement] Selection after type change needs to follow HTML specification
Product: WebKit Reporter: zsun
Component: FormsAssignee: zsun
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, esprehn+autocc, ews-watchlist, gyuyoung.kim, mifenton, ntim, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Comment 1 zsun 2022-03-02 06:16:39 PST
Created attachment 453600 [details]
Patch
Comment 2 zsun 2022-03-02 06:17:47 PST
Not ready for review yet. I need to rebase the test expectations across platforms.
Comment 3 Radar WebKit Bug Importer 2022-03-09 05:57:17 PST
<rdar://problem/90027274>
Comment 4 zsun 2022-04-27 03:10:25 PDT
Created attachment 458437 [details]
Patch
Comment 5 zsun 2022-04-28 02:11:34 PDT
Created attachment 458502 [details]
Patch
Comment 6 zsun 2022-04-28 02:37:03 PDT
Created attachment 458505 [details]
Patch
Comment 7 zsun 2022-04-28 02:45:44 PDT
Created attachment 458506 [details]
Patch
Comment 8 zsun 2022-04-28 04:39:46 PDT
Created attachment 458511 [details]
Patch
Comment 9 zsun 2022-04-28 05:42:06 PDT
Created attachment 458520 [details]
Patch
Comment 10 Chris Dumez 2022-04-28 11:00:21 PDT
Comment on attachment 458520 [details]
Patch

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

> Source/WebCore/html/HTMLTextFormControlElement.cpp:97
> +    m_cachedSelectionDirection = document.frame() && document.frame()->editor().behavior().shouldConsiderSelectionAsDirectional() ? SelectionHasForwardDirection: SelectionHasNoDirection;

Why did you move this out of the initializer list? It seems like it could / should stay in the initializer list.
Comment 11 zsun 2022-04-29 01:33:03 PDT
Created attachment 458575 [details]
Patch
Comment 12 Tim Nguyen (:ntim) 2022-05-01 02:55:17 PDT
Comment on attachment 458575 [details]
Patch

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

> Source/WebCore/html/HTMLTextFormControlElement.cpp:91
> +    , m_cachedSelectionDirection(document.frame() && document.frame()->editor().behavior().shouldConsiderSelectionAsDirectional() ? SelectionHasForwardDirection: SelectionHasNoDirection)

nit: please add a space after `SelectionHasForwardDirection`
Comment 13 zsun 2022-05-02 00:04:38 PDT
Created attachment 458664 [details]
Patch
Comment 14 Tim Nguyen (:ntim) 2022-05-02 11:37:48 PDT
Comment on attachment 458664 [details]
Patch

Why does imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/selection-start-end-extra-expected.txt need different expectations per-platform?
Comment 15 zsun 2022-05-03 01:44:29 PDT
(In reply to Tim Nguyen (:ntim) from comment #14)
> Comment on attachment 458664 [details]
> Patch
> 
> Why does
> imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/
> selection-start-end-extra-expected.txt need different expectations
> per-platform?

This patch fixes the sub-test "Shortening value by turning the input type into 'color' and back to 'text'" in selection-start-end-extra.html for all the platforms except mac-wk1.
Comment 16 Tim Nguyen (:ntim) 2022-05-03 04:59:53 PDT
(In reply to zsun from comment #15)
> (In reply to Tim Nguyen (:ntim) from comment #14)
> > Comment on attachment 458664 [details]
> > Patch
> > 
> > Why does
> > imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/
> > selection-start-end-extra-expected.txt need different expectations
> > per-platform?
> 
> This patch fixes the sub-test "Shortening value by turning the input type
> into 'color' and back to 'text'" in selection-start-end-extra.html for all
> the platforms except mac-wk1.

If that's the case, can we make the failing expectation specific to mac-wk1 and just update the common expectation for all the other platforms?
Comment 17 zsun 2022-05-03 05:20:27 PDT
(In reply to Tim Nguyen (:ntim) from comment #16)
> (In reply to zsun from comment #15)
> > (In reply to Tim Nguyen (:ntim) from comment #14)
> > > Comment on attachment 458664 [details]
> > > Patch
> > > 
> > > Why does
> > > imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/
> > > selection-start-end-extra-expected.txt need different expectations
> > > per-platform?
> > 
> > This patch fixes the sub-test "Shortening value by turning the input type
> > into 'color' and back to 'text'" in selection-start-end-extra.html for all
> > the platforms except mac-wk1.
> 
> If that's the case, can we make the failing expectation specific to mac-wk1
> and just update the common expectation for all the other platforms?

This was my initial thought at https://bugs.webkit.org/attachment.cgi?id=458520&action=diff but mac-wk1 didn't seem picking up the mac-wk1 expectation file.
Comment 18 Tim Nguyen (:ntim) 2022-05-03 10:01:45 PDT
(In reply to zsun from comment #17)
> (In reply to Tim Nguyen (:ntim) from comment #16)
> > (In reply to zsun from comment #15)
> > > (In reply to Tim Nguyen (:ntim) from comment #14)
> > > > Comment on attachment 458664 [details]
> > > > Patch
> > > > 
> > > > Why does
> > > > imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/
> > > > selection-start-end-extra-expected.txt need different expectations
> > > > per-platform?
> > > 
> > > This patch fixes the sub-test "Shortening value by turning the input type
> > > into 'color' and back to 'text'" in selection-start-end-extra.html for all
> > > the platforms except mac-wk1.
> > 
> > If that's the case, can we make the failing expectation specific to mac-wk1
> > and just update the common expectation for all the other platforms?
> 
> This was my initial thought at
> https://bugs.webkit.org/attachment.cgi?id=458520&action=diff but mac-wk1
> didn't seem picking up the mac-wk1 expectation file.

Looks like it didn't work because your paths don't match up. You put the expectation in:

LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/selection-start-end-extra-expected.txt

when it should be in:

LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/selection-start-end-extra-expected.txt
Comment 19 zsun 2022-05-04 02:51:42 PDT
Created attachment 458784 [details]
Patch
Comment 20 zsun 2022-05-04 02:53:17 PDT
(In reply to Tim Nguyen (:ntim) from comment #18)
> (In reply to zsun from comment #17)
> > (In reply to Tim Nguyen (:ntim) from comment #16)
> > > (In reply to zsun from comment #15)
> > > > (In reply to Tim Nguyen (:ntim) from comment #14)
> > > > > Comment on attachment 458664 [details]
> > > > > Patch
> > > > > 
> > > > > Why does
> > > > > imported/w3c/web-platform-tests/html/semantics/forms/textfieldselection/
> > > > > selection-start-end-extra-expected.txt need different expectations
> > > > > per-platform?
> > > > 
> > > > This patch fixes the sub-test "Shortening value by turning the input type
> > > > into 'color' and back to 'text'" in selection-start-end-extra.html for all
> > > > the platforms except mac-wk1.
> > > 
> > > If that's the case, can we make the failing expectation specific to mac-wk1
> > > and just update the common expectation for all the other platforms?
> > 
> > This was my initial thought at
> > https://bugs.webkit.org/attachment.cgi?id=458520&action=diff but mac-wk1
> > didn't seem picking up the mac-wk1 expectation file.
> 
> Looks like it didn't work because your paths don't match up. You put the
> expectation in:
> 
> LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/semantics/
> forms/the-input-element/selection-start-end-extra-expected.txt
> 
> when it should be in:
> 
> LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/semantics/
> forms/textfieldselection/selection-start-end-extra-expected.txt

Oops! I was so careless. Thank you!
Comment 21 Tim Nguyen (:ntim) 2022-05-04 06:25:39 PDT
Comment on attachment 458784 [details]
Patch

Thanks for addressing the comments! I assume this is ready to land.
Comment 22 EWS 2022-05-04 07:49:37 PDT
Committed r293773 (250252@main): <https://commits.webkit.org/250252@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 458784 [details].
Comment 23 zsun 2022-05-10 01:19:50 PDT
*** Bug 239769 has been marked as a duplicate of this bug. ***