1297635 - Add a helper function for input to check whether input events with modifier should change the value

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Ming-Chou Shih [:stone]]

Now we have different key modifiers checks when handling key events for range and number input. We should align them if possible.

Comment 1

[Ming-Chou Shih [:stone]]

Attachment: Add a helper function for input to check whether input events with modifier should change the value (V1)

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8789187 [details] [diff] [review]
Add a helper function for input to check whether input events with modifier should change the value (V1)

Review of attachment 8789187 [details] [diff] [review]:
-----------------------------------------------------------------

f=me after the following issue is addressed, thanks!

::: dom/html/HTMLInputElement.cpp
@@ +8326,5 @@
>    aSequence.AppendElements(mEntries);
>  }
>  
> +bool
> +HTMLInputElement::IgnoreInputEventWithModifier(WidgetInputEvent* aEvent) const

/* static */ bool
HTMLInputElement::IsInputModifier(WidgetInputEvent* aEvent) const

::: dom/html/HTMLInputElement.h
@@ +1518,5 @@
>  
> +  /**
> +   * Return true if the input event should be ignore because of it's modifiers
> +   */
> +  bool IgnoreInputEventWithModifier(WidgetInputEvent* aEvent) const;

This is expected to be a static utility instead of a instance member function of this class.
Let rename it to static bool IsInputModifier(WidgetInputEvent* aEvent) const;

BTW, this is out of the scope of this bug but it's even better if we can have these utilities in the the anonymous namespace in .cpp to prevent adding this private functions in the .h which is not used by other modules.

Comment 3

[Ming-Chou Shih [:stone]]

Attachment: Add a helper function for input to check whether input events with modifier should change the value (V2)

Comment 4

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

Comment on attachment 8790134 [details] [diff] [review]
Add a helper function for input to check whether input events with modifier should change the value (V2)

We might even want to add a method similar to this to WidgetInputEvent, 
but this is fine too.

Comment 5

[Ming-Chou Shih [:stone]]

Attachment: Add a helper function for input to check whether input events with modifier should change the value (V3)

Updated the patch summary

Comment 6

[Ming-Chou Shih [:stone]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=418f157188dd9bb867872415570405786d8a0cc1

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/be466c64f9c0
Add a helper function for input to check whether input events with modifier should change the value. r=smaug, f=bevistseng

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/be466c64f9c0