Bug 254761

Summary: input type=range not using [value] for step base
Product: WebKit Reporter: Carlos Lopez <clshortfuse>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ahmad.saleem792, akeerthi, cdumez, karlcow, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: BrowserCompat, InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
rendering in safari, firefox, chrome none

Description Carlos Lopez 2023-03-30 12:29:50 PDT 
CodePen: https://codepen.io/shortfuse/pen/xxaNpbR?editors=1010

Spec: 

> The step base is the value returned by the following algorithm:

> If the element has a min content attribute, and the result of applying the algorithm to convert a string to a number to the value of the min content attribute is not an error, then return that result.

> If the element has a value content attribute, and the result of applying the algorithm to convert a string to a number to the value of the value content attribute is not an error, then return that result.

> If a default step base is defined for this element given its type attribute's state, then return it.

> Return zero.

https://html.spec.whatwg.org/multipage/input.html#the-step-attribute

From my understanding, if [min] *attribute* is not present, the step should be the [value] attribute. But it appears Webkit is using the min fallback value (default 0). This will also change the value of the control:

<input type=range step=10 value=1> has computed value of 0, not 1.

Neither Chrome, nor Firefox show this issue:

See: https://bugs.chromium.org/p/chromium/issues/detail?id=339194
Comment 2 Karl Dubost 2023-04-02 17:29:47 PDT 
Created attachment 465736 [details]
rendering in safari, firefox, chrome

Tested with safari stp 166
Comment 3 Radar WebKit Bug Importer 2023-04-06 12:30:18 PDT 
<rdar://problem/107721910>
Comment 4 Ahmad Saleem 2023-09-01 15:56:04 PDT 
Adding following function definition in InputType.h:

Decimal findStepBase(const Decimal&) const;

and then following in InputType.cpp:

Decimal InputType::findStepBase(const Decimal& defaultValue) const
{
    Decimal stepBase = parseToNumberOrNaN(element()->attributeWithoutSynchronization(minAttr));
    if (!stepBase.isFinite())
        stepBase = parseToNumber(element()->attributeWithoutSynchronization(valueAttr), defaultValue);
    return stepBase;
}

and then merging first Blink patch from linked bug fixes this bug:

Below: https://src.chromium.org/viewvc/blink?view=revision&revision=166367

NOTE: There is assertion issue later in 'StepRange' so fixing it in one go: https://chromium.googlesource.com/chromium/src.git/+/807ab32fd2e5accda8c5cef2678e0e0af23158b0

____

Changes from Blink patch:

> StepRange.cpp (in function: clampValue())

// Rounds inRangeValue to stepBase + N * step.
    const Decimal roundedValue = roundByStep(inRangeValue, m_stepBase);
    const Decimal clampedValue = roundedValue > m_maximum ? roundedValue - m_step : (roundedValue < m_minimum ? roundedValue + m_step : roundedValue);

and ASSERT fix:

// clampedValue can be outside of [m_minimum, m_maximum] if m_step is huge.
    if (clampedValue < m_minimum || clampedValue > m_maximum)
        return inRangeValue;

and then:

> RangeInputType.cpp (in function: supportsRequired())

const Decimal stepBase = findStepBase(rangeDefaultStepBase);

and updating return:

return StepRange(stepBase, RangeLimitations::Valid, minimum, maximum, step, rangeStepDescription);

____

Still working locally to merge other Blink commit as well to fix this in one go but still wanted to write down progress.
Comment 5 Ahmad Saleem 2023-09-01 17:52:05 PDT 
OK, just from quick analysis from Blink's second commit, I think we don't need to do all changes but just 'InputType.cpp' and 'InputType.h'. Will test further tomorrow. :-)
Comment 6 Ahmad Saleem 2023-09-02 05:12:59 PDT 
NOTE - this will introduce bug and have follow-up commit as well: https://chromium.googlesource.com/chromium/src.git/+/1ed75b7595d0e227e35d561878ffb364253f3784
Comment 7 Karl Dubost 2023-09-03 16:55:23 PDT 
I wonder if there are missing WPT tests covering these cases about the value.
 
Maybe in constraints
https://wpt.fyi/results/html/semantics/forms/constraints?label=master&label=experimental&aligned



The test of the CodePen is:

<input id="control1" type="range" step="10" value="1">

document.getElementById('control1').attributes.value.textContent returns "1"
document.getElementById('control1').value returns "0"
Comment 8 EWS 2024-06-18 10:13:42 PDT 
Committed 280127@main (5b73f8e46d3a): <https://commits.webkit.org/280127@main>

Reviewed commits have been landed. Closing PR #22889 and removing active labels.