Fix #74

[romulocintra]

closes #74

[sffc]

Hmm. Before, we had

        1. Let _value_ be ? Get(_options_, _property_).
        1. If _value_ is *undefined*, return _fallback_.
        1. If _value_ is *true*, return _trueValue_.
        1. Let _valueBoolean_ be ToBoolean(_value_).
        1. If _valueBoolean_ is *false*, return _falsyValue_.
        1. Let _value_ be ? ToString(_value_).
        1. If _values_ does not contain an element equal to _value_, throw a *RangeError* exception.
        1. Return _value_.

What if we just change the exception to return _trueValue_, like

        1. Let _value_ be ? Get(_options_, _property_).
        1. If _value_ is *undefined*, return _fallback_.
        1. If _value_ is *true*, return _trueValue_.
        1. Let _valueBoolean_ be ToBoolean(_value_).
        1. If _valueBoolean_ is *false*, return _falsyValue_.
        1. Let _value_ be ? ToString(_value_).
        1. If _values_ does not contain an element equal to _value_, return _trueValue_.
        1. Return _value_.

[romulocintra]

What if we just change the exception to return _trueValue_, like

        1. Let _value_ be ? Get(_options_, _property_).
        1. If _value_ is *undefined*, return _fallback_.
        1. If _value_ is *true*, return _trueValue_.
        1. Let _valueBoolean_ be ToBoolean(_value_).
        1. If _valueBoolean_ is *false*, return _falsyValue_.
        1. Let _value_ be ? ToString(_value_).
        1. If _values_ does not contain an element equal to _value_, return _trueValue_.
        1. Return _value_.

In that case, all strings not contained in list of valid ones will have trueValue behaviour of "always" that might be confusing due to that defaultGrouping it's auto. And how about the use case string "false"?

[sffc]

OK, how about

        1. Let _value_ be ? Get(_options_, _property_).
        1. If _value_ is *undefined*, return _fallback_.
        1. If _value_ is *true*, return _trueValue_.
        1. Let _valueBoolean_ be ToBoolean(_value_).
        1. If _valueBoolean_ is *false*, return _falsyValue_.
        1. Let _value_ be ? ToString(_value_).
        1. If _values_ does not contain an element equal to _value_, return _fallback_.
        1. Return _value_.

This retains the current web reality behavior because true is currently just "auto".

[romulocintra]

Updated the PR addressing your comments and LGTM,

OK, how about

        1. Let _value_ be ? Get(_options_, _property_).
        1. If _value_ is *undefined*, return _fallback_.
        1. If _value_ is *true*, return _trueValue_.
        1. Let _valueBoolean_ be ToBoolean(_value_).
        1. If _valueBoolean_ is *false*, return _falsyValue_.
        1. Let _value_ be ? ToString(_value_).
        1. If _values_ does not contain an element equal to _value_, return _fallback_.
        1. Return _value_.

This retains the current web reality behavior because true is currently just "auto".

This way there is no risk of breaking current web behaviour and the issue it's solved, on the other hand, throwing an error makes it more consistent with GetOption and other methods that use it.

[sffc]

The current behavior is

Let _useGrouping_ be ? StringOrBooleanOption(options, "useGrouping", "boolean", undefined, true).

This never fails, because the undefined means "accept all values".

[sffc]

@anba @FrankYFTang @constellation Take note of this change. useGrouping should not throw a RangeError any more on malformed arguments.

[FrankYFTang]

so.. now it will not only accept {'useGrouping': 'true'} but also {'useGrouping': 'HelloWorld'} ? Isn't that too loose?

[sffc]

The issue is that it already accepts {'useGrouping': 'HelloWorld'}

> new Intl.NumberFormat("en", {useGrouping: "HelloWorld"}).resolvedOptions().useGrouping
true

So this is a web compat fix.