Bug 156558

Summary: :in-range & :out-of-range CSS pseudo-classes shouldn't match inputs without range limitations
Product: WebKit Reporter: Chris Rebert <webkit>
Component: CSSAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, dino, koivisto, rniwa
Priority: P2 Keywords: HasReduction, W3CTest
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
URL: http://w3c-test.org/html/semantics/selectors/pseudo-classes/inrange-outofrange.html
Attachments:
Description Flags
Testcase demonstrating the bug
none
Screenshot of testcase on iOS
none
Patch
none
Patch
none
Patch for landing none

Description Chris Rebert 2016-04-13 15:09:39 PDT
Created attachment 276361 [details]
Testcase demonstrating the bug

Steps to reproduce the problem:
1. Open the attached testcase in WebKit Nightly on OS X or in iOS 9.3 Safari.
2. Observe the background-colors and borders of the <input>s

What is the expected behavior?
The inputs should have neither a red border nor a red background-color,
because they don't have range limitations ( https://html.spec.whatwg.org/multipage/forms.html#have-range-limitations ),
because they have neither `min` nor `max` attributes
and the HTML spec doesn't define a default minimum or maximum for any of these input types.
Therefore, these inputs should fail the 2nd of the 3 conditions required to match :in-range or :out-of-range .

Per https://html.spec.whatwg.org/multipage/scripting.html#selector-in-range
> The :in-range pseudo-class must match all elements that are [...], have range limitations, and [...]

What went wrong?
The inputs have red borders and/or red backgrounds, indicating that they matched the :in-range or :out-of-range pseudo-classes despite not having range limitations.
Comment 1 Chris Rebert 2016-04-13 15:14:00 PDT
Created attachment 276362 [details]
Screenshot of testcase on iOS
Comment 2 Benjamin Poulain 2016-06-15 17:12:32 PDT
Created attachment 281406 [details]
Patch
Comment 3 Benjamin Poulain 2016-06-15 19:01:49 PDT
Created attachment 281423 [details]
Patch
Comment 4 Simon Fraser (smfr) 2016-06-16 14:21:57 PDT
Comment on attachment 281423 [details]
Patch

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

> Source/WebCore/html/InputType.cpp:343
> +        return false;

Shouldn't this return true? If the input has no range limitations, aren't you aways in-range?

> Source/WebCore/html/NumberInputType.cpp:170
> +        const AtomicString& minAttribute = element.fastGetAttribute(minAttr);
> +        Decimal minimumFromAttribute = parseToNumberOrNaN(minAttribute);
> +        if (minimumFromAttribute.isFinite()) {
> +            hasRangeLimitations = true;
> +            minimum = minimumFromAttribute;
> +        }

I think this could go into a separate function or lambda and be shared.

> Source/WebCore/html/StepRange.h:74
> +    StepRange(const Decimal& stepBase, bool hasRangeLimitations, const Decimal& minimum, const Decimal& maximum, const Decimal& step, const StepDescription&);

Please make an enum for hasRangeLimitations. The call sites are unreadable with this change.
Comment 5 Benjamin Poulain 2016-06-16 14:34:33 PDT
(In reply to comment #4)
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281423&action=review
> 
> > Source/WebCore/html/InputType.cpp:343
> > +        return false;
> 
> Shouldn't this return true? If the input has no range limitations, aren't
> you aways in-range?

The way the spec defines it, you are only in range if there is a valid range to begin with.
In this case, since you have no range or and invalid range, you are not in-range.
Comment 6 Benjamin Poulain 2016-06-16 15:25:50 PDT
Created attachment 281480 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2016-06-16 15:47:03 PDT
Comment on attachment 281480 [details]
Patch for landing

Clearing flags on attachment: 281480

Committed r202143: <http://trac.webkit.org/changeset/202143>
Comment 8 WebKit Commit Bot 2016-06-16 15:47:07 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Chris Rebert 2016-06-16 16:05:31 PDT
Wonderful! I look forward to the next Tech Preview release so that I can update http://caniuse.com/#feat=css-in-out-of-range to reflect this fix.
Comment 10 Benjamin Poulain 2016-06-16 16:12:11 PDT
(In reply to comment #9)
> Wonderful! I look forward to the next Tech Preview release so that I can
> update http://caniuse.com/#feat=css-in-out-of-range to reflect this fix.

Your bug report was really high quality. Thanks a lot for taking the time to write a test case and highlight the relevant spec.