851782 - Dispatch an 'input' event when the 'type' of an <input type=range> is changed while a user is dragging its thumb

Status: RESOLVED FIXED

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

Description

[Jonathan Watt [:jwatt]]

It is possible in one rather contorted scenario for nsHTMLInputElement::SetValueOfRangeForUserEvent to dispatch a trusted "input" event under a change that was initiated by content script. Specifically this will happen if script changes the type of the <input> from "range" to something else while the user is in the middle of dragging the thumb. We then end up with a stack like this:

  nsHTMLInputElement::SetValueOfRangeForUserEvent
  nsHTMLInputElement::CancelRangeThumbDrag
  nsHTMLInputElement::HandleTypeChange
  nsHTMLInputElement::ParseAttribute
  mozilla::dom::Element::SetAttr
  nsGenericHTMLElement::SetAttr
  nsGenericHTMLElement::SetAttr
  nsGenericHTMLElement::SetAttrHelper
  nsHTMLInputElement::SetType
  nsIDOMHTMLInputElement_SetType

Really we should only call nsHTMLInputElement::SetValueOfRangeForUserEvent for an action being carried out by the user. However, it does seem like we need to dispatch an 'input' event in this case, or else the last action carried out by any 'input' handler in the page will have been for a value that isn't the final value, leaving it in a bad state. (We also may not fire a 'change' event, since cancelling the drag means that the value will be the same as the value at the start of the drag.)

Comment 1

[Jonathan Watt [:jwatt]]

To be clearer, a fuller stack where we hit the assertion that resulted in this bug is:

###!!! ASSERTION: This is unsafe! Fix the caller!: 'Error', file ../../../../content/events/src/nsEventDispatcher.cpp, line 494
nsEventDispatcher::Dispatch
nsEventDispatcher::DispatchDOMEvent
nsINode::DispatchEvent
nsContentUtils::DispatchEvent
nsContentUtils::DispatchTrustedEvent
nsHTMLInputElement::SetValueOfRangeForUserEvent
nsHTMLInputElement::CancelRangeThumbDrag
nsHTMLInputElement::HandleTypeChange
nsHTMLInputElement::ParseAttribute
mozilla::dom::Element::SetAttr
nsGenericHTMLElement::SetAttr
nsGenericHTMLElement::SetAttr
nsGenericHTMLElement::SetAttrHelper
nsHTMLInputElement::SetType
nsIDOMHTMLInputElement_SetType

Comment 2

[Jonathan Watt [:jwatt]]

The fix pushed in bug 851090 comment 15 stopped us from dispatching 'input' events at times that would trigger the assertion. So this bug is now about dispatching 'input' events when a drag is canceled by a script changing the 'type' attribute, but doing it in a safe way.

Comment 3

[Jonathan Watt [:jwatt]]

As pointed out by smaug on IRC, this really is a pretty far out edge case, so probably it doesn't really block bug 841950. Changing it to block bug 853822 instead.

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: patch

Nevertheless, here's a patch to fix this.

Comment 5

[Jonathan Watt [:jwatt]]

I ran test_input_range_mouse_and_touch_events.html with this patch, and it now works fine.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 737974 [details] [diff] [review]
patch

New patch coming

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: patch

patch with the setting of mIsDraggingRange moved up to happen before the event dispatch.

Comment 8

[Jonathan Watt [:jwatt]]

Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/8bae422aed39

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8bae422aed39