Bug 254761 – input type=range not using [value] for step base

• Component: Forms
• Status: RESOLVED FIXED

[Carlos Lopez]

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

[Karl Dubost]

https://searchfox.org/wubkat/rev/64453e226bbd56f49b248f0f8816a72e5547e456/Source/WebCore/html/InputType.cpp#1103

The logic for StepUp

[Karl Dubost]

Created attachment 465736 [details]
rendering in safari, firefox, chrome

Tested with safari stp 166

[Radar WebKit Bug Importer]

<rdar://problem/107721910>

[Ahmad Saleem]

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.

[Ahmad Saleem]

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. :-)

[Ahmad Saleem]

NOTE - this will introduce bug and have follow-up commit as well: https://chromium.googlesource.com/chromium/src.git/+/1ed75b7595d0e227e35d561878ffb364253f3784

[Karl Dubost]

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"

[EWS]

Committed 280127@main (5b73f8e46d3a): <https://commits.webkit.org/280127@main>

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