Bug 249771 - Fix Decimal.floor() + Decimal.ceiling() for many non-integral values
Summary: Fix Decimal.floor() + Decimal.ceiling() for many non-integral values
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2022-12-22 06:32 PST by Ahmad Saleem
Modified: 2024-04-06 09:49 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2022-12-22 06:32:35 PST
Hi Team,

While going through Blink's commit, I came across another failing test case:

Test Case - <input type="number" min="1.3" value="1.51" step="0.2"/>

^ Try to do step-up in Safari, it will first go from 1.51 to 1.5 and then 1.7 while Chrome Canary 111 and Firefox Nightly 110 works perfectly fine.

Blink Commit - https://src.chromium.org/viewvc/blink?view=revision&revision=162197

WebKit GitHub Source - https://github.com/WebKit/WebKit/blob/dced9d40e0ba387b976739efb45a6513be9bcd4a/Source/WebCore/platform/Decimal.cpp#L622

Just wanted to raise it so we can fix it later.

Thanks!
Comment 1 Ahmad Saleem 2022-12-23 02:02:39 PST
Manage to have testcase - will try to do PR later today - https://jsfiddle.net/mwn0jy35/
Comment 2 Radar WebKit Bug Importer 2022-12-29 06:33:15 PST
<rdar://problem/103759613>
Comment 3 Ahmad Saleem 2023-01-17 15:32:13 PST
I am closing this PR for time being because I noted that we still have other pre-requisite bugs to fix in StepUp and StepDown and closing my PR for time being with following comment:

''

NOTE to myself - I am going to close this PR since there are still bugs in StepUp and StepDown, which we should fix since this might be causing this to fail. So I will look into this later, once I fix other bugs.

''


PR - https://github.com/WebKit/WebKit/pull/8045
Comment 4 Ahmad Saleem 2024-04-06 09:49:21 PDT
Just copying that following compiles when added to 'Decimal.cpp' (for TestWebKit API):

// Simulate core/html/forms/StepRange
class DecimalStepRange {

public:
    Decimal maximum;
    Decimal minimum;
    Decimal step;

    DecimalStepRange(const Decimal& minimum, const Decimal& maximum, const Decimal& step)
        : maximum(maximum)
        , minimum(minimum)
        , step(step)
    {
    }

    Decimal clampValue(Decimal value) const
    {
        const Decimal result = minimum + ((value - minimum) / step).round() * step;
        ASSERT(result.isFinite());
        return result > maximum ? result - step : result;
    }
};

class Decimcal : public testing::Test {
protected:
    using Sign = Decimal::Sign;
    static constexpr Sign positive = Decimal::Positive;
    static constexpr Sign negative = Decimal::Negative;

    Decimal encode(uint64_t coefficient, int exponent, Sign sign)
    {
        return Decimal(sign, exponent, coefficient);
    }

    Decimal fromString(const String& string)
    {
        return Decimal::fromString(string);
    }

    Decimal stepDown(const String& minimum, const String& maximum, const String& step, const String& valueString, int numberOfStepTimes)
    {
        DecimalStepRange stepRange(fromString(minimum), fromString(maximum), fromString(step));
        Decimal value = fromString(valueString);

        for (int i = 0; i < numberOfStepTimes; ++i) {
            value -= stepRange.step;
            value = stepRange.clampValue(value);
        }
        return value;
    }

    Decimal stepUp(const String& minimum, const String& maximum, const String& step, const String& valueString, int numberOfStepTimes)
    {
        DecimalStepRange stepRange(fromString(minimum), fromString(maximum), fromString(step));
        Decimal value = fromString(valueString);

        for (int i = 0; i < numberOfStepTimes; ++i) {
            value += stepRange.step;
            value = stepRange.clampValue(value);
        }
        return value;
    }
};