Closed Bug 893332 Opened 7 years ago Closed 4 years ago

<input type=range> — "Assertion failure: !GetValidityState(VALIDITY_STATE_RANGE_UNDERFLOW) (HTML5 spec does not allow this)"


(Core :: DOM: Core & HTML, defect)

Not set



Tracking Status
firefox47 --- fixed


(Reporter: jruderman, Assigned: jwatt)


(Blocks 2 open bugs)


(Keywords: assertion, testcase)


(6 files)

Attached file testcase
Assertion failure: !GetValidityState(VALIDITY_STATE_RANGE_UNDERFLOW) (HTML5 spec does not allow this), at content/html/content/src/HTMLInputElement.cpp:1103
Attached file stack
I am getting this when running a WebGL demo in a DEBUG build.
jwatt, do you have comments on this?  Even though it's not allowed by the spec, it does seem to be happening in the real world, and it looks like our internal state could be inconsistent because of this.
Flags: needinfo?(jwatt)
Jesse's attached testcase now hits a different MOZ_ASSERT prior to hitting the "HTML5 spec does not allow this" assert, so given that the more recent issues hit the "HTML5 spec does not allow this" first there seem to be two different issues.
Flags: needinfo?(jwatt)
In Jesse's testcase, when the 'min' attribute is set we need to increase the value of the input to the minimum value of the input. We do that under:


It looks like the SetValueInternal call that does this is happening after the UpdateRangeUnderflowValidityState though, and so the underflow validity state is set based on the input's value before it changes.
With the above fixed, another problem is that in HTMLInputElement::SanitizeValue here:

  DebugOnly<bool> ok = value.toString(buf, ArrayLength(buf));

|value| has the values:

  m_coefficient	uint64_t	1000000000000003	1000000000000003
  m_exponent	int16_t	-11	-11

but the toString() call sets |buf| to "10000" instead of "10000.00000000003".

The result of this is that the inputs minimum value is "10000.00000000003" but its value is updated to "10000", and hence the input is still underflowing.
The reason toString() sets |buf| to "10000" is because in this code:

DBL_DIG==15, so a couple of the least significant digits get chopped off by that loop.
(In reply to Jeff Gilbert [:jgilbert] from comment #2)
> I am getting this when running a WebGL demo in a DEBUG build.

That page reduces to:

data:text/html,<input id="gravitySlider" min="-10" max="10" step="0.1" type="range" value="-5"/>

In this case the error is happening during parse time. The parser sets the attributes in reverse, so we have the value set to -5, then when the step is set we end up calling GetValidityState while mParserCreating is true:

Since we haven't finished parsing yet, HTMLInputElement::DoneCreatingElement hasn't yet sanitized the value and it is invalid to call GetValidityState (i.e. the asserts are buggy).
(In reply to Carsten Book [:Tomcat] from comment #3)
> also found by bughunter on
> jackets?page=2&size=36&cols=4&sort=&id=/womens/shop-by-category/coats-and-
> jackets&priceRange%5Bmin%5D=7&priceRange%5Bmax%5D=55

And this one reduced to:

data:text/html,<input max="50" min="10" step="2" type="range" value="7"/>

and is the same as the issue described in the previous comment (it happens during parse time, the value is set to 7 followed by min being set to 10, at which point the assertion in the |aName == nsGkAtoms::min| block fails).
(These parse time assertion failures do not result from an actual bug, since as noted above HTMLInputElement::DoneCreatingElement sanitizes.)
Assignee: nobody → jwatt
Attachment #8712743 - Flags: review?(amarchesini)
Comment on attachment 8712743 [details] [diff] [review]
part 1 - Rename misnamed SetValidityState/GetValidityState arguments

Review of attachment 8712743 [details] [diff] [review]:

::: dom/html/nsIConstraintValidation.h
@@ +68,1 @@
>                          bool mValue);

mValue -> aValue

@@ +83,4 @@
>    nsresult CheckValidity(bool* aValidity);
>    void     SetCustomValidity(const nsAString& aError);
> +  bool GetValidityState(ValidityStateType aState) const {

{ in a new line.
Attachment #8712743 - Flags: review?(amarchesini) → review+
Attachment #8712747 - Flags: review?(amarchesini) → review+
Attachment #8712898 - Flags: review?(amarchesini)
Gah. I've been playing with git and meant to push from my git tree to a local inbound tree. Instead I managed to push directly to inbound. Sorry about that, baku. I'll push follow-up commits if you have any review comments that need to be addressed.
These patches fix the in-the-wild cases that have been found, but not Jesse's original testcase since that is a rounding issue in the Decimal code. I'm a lot less concerned about Jesse's case though since it's a lot rarer that content authors will be writing numbers with so many significant digits for a range control. Rather than keep this bug open I'll clone this bug to deal with Jesse's original testcase separately.
Blocks: 1243560
Attachment #8712897 - Flags: review?(amarchesini) → review+
Attachment #8712898 - Flags: review?(amarchesini) → review+
You need to log in before you can comment on or make changes to this bug.