[Forms] Value doesn't change for stepUp()/stepDown() with out of range values

• Component: Forms
• Status: RESOLVED FIXED

Currently WebKit throws "invalid state" for these cases. As per section 10 in https://html.spec.whatwg.org/multipage/input.html#dom-input-stepup, we should do "return", which  is to indicate that value doesn't change.

Created attachment 449807 [details]
Patch

There are a few tests failed in fast/forms/, these tests are expecting "invalid state".

I guess it probably makes sense to reach an agreement to use "return {}" rather than "return Exception { InvalidStateError }" for out of range for stepup() and stepdown() first, before putting effort on updating these tests?

Any comments? Thank you!

[Radar WebKit Bug Importer]

<rdar://problem/88266239>

Created attachment 450765 [details]
Patch

[Chris Dumez]

Comment on attachment 450765 [details]
Patch

Looks like there are some legit failures on WK1: https://ews-build.s3-us-west-2.amazonaws.com/macOS-Catalina-Release-WK1-Tests-EWS/r450765-25455/results.html

Created attachment 450770 [details]
Patch

[Chris Dumez]

Comment on attachment 450770 [details]
Patch

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

> LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/input-stepdown-expected.txt:7
> +FAIL Calling stepDown() on input - date - where value < min should not modify value. The object is in an invalid state.

Why is WK1 behaving differently here? The code running should be the same.

I suspect those WK1 baselines are wrong.

> LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/input-stepup-expected.txt:8
> +FAIL Calling stepUp() on input -date- where value > max should not modify value. The object is in an invalid state.

ditto.

[Chris Dumez]

Comment on attachment 450770 [details]
Patch

r=me. Based on EWS output, those Mac-wk1 baselines are correct so r+. That said, I am kinda curious why our behavior is worse on WK1 here.

(In reply to Chris Dumez from comment #8)
> Comment on attachment 450770 [details]
> Patch
> 
> r=me. Based on EWS output, those Mac-wk1 baselines are correct so r+.

Thanks very much for the review!

> I am kinda curious why our behavior is worse on WK1 here.

Probably not "worse". One sub-test (number) is now passing for WK1 while for other platforms all underflow/overflow sub-tests are passing.

[Chris Dumez]

(In reply to zsun from comment #9)
> (In reply to Chris Dumez from comment #8)
> > Comment on attachment 450770 [details]
> > Patch
> > 
> > r=me. Based on EWS output, those Mac-wk1 baselines are correct so r+.
> 
> Thanks very much for the review!
> 
> > I am kinda curious why our behavior is worse on WK1 here.
> 
> Probably not "worse". One sub-test (number) is now passing for WK1 while for
> other platforms all underflow/overflow sub-tests are passing.

I am not saying worse because of your patch. However, it seems WK2 got more new PASSes from your patch than WK1 no?

(In reply to Chris Dumez from comment #10)
> (In reply to zsun from comment #9)
> > (In reply to Chris Dumez from comment #8)
> > > Comment on attachment 450770 [details]
> > > Patch
> > > 
> > > r=me. Based on EWS output, those Mac-wk1 baselines are correct so r+.
> > 
> > Thanks very much for the review!
> > 
> > > I am kinda curious why our behavior is worse on WK1 here.
> > 
> > Probably not "worse". One sub-test (number) is now passing for WK1 while for
> > other platforms all underflow/overflow sub-tests are passing.
> 
> I am not saying worse because of your patch. However, it seems WK2 got more
> new PASSes from your patch than WK1 no?

Yes, you are right :)

[EWS]

Committed r289075 (246781@main): <https://commits.webkit.org/246781@main>

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

[Sam Weinig]

Reopening to attach new patch.

[Sam Weinig]

Created attachment 450847 [details]
Patch

(In reply to Sam Weinig from comment #14)
> Created attachment 450847 [details]
> Patch

Sam, is the patch you attached related to this issue?

[Chris Dumez]

Comment on attachment 450847 [details]
Patch

He uploaded to the wrong bug.

[Brent Fulgham]

Moving back to Resolved/Fixed.

[Tim Nguyen (:ntim)]

*** Bug 233721 has been marked as a duplicate of this bug. ***

[[e-mail]]

*** Bug 192587 has been marked as a duplicate of this bug. ***