Closed Bug 1297635 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: stone, Assigned: stone)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Now we have different key modifiers checks when handling key events for range and number input. We should align them if possible.
Assignee: nobody → sshih
Depends on: 1295719
Summary: Add helper function for input to check whether key event should change the value → Add a helper function for input to check whether input events with modifier should change the value
Attachment #8789187 - Flags: feedback?(btseng)
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.
Attachment #8789187 - Flags: feedback?(btseng) → feedback+
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.
Attachment #8790134 - Flags: review?(bugs) → review+
Updated the patch summary
Attachment #8790134 - Attachment is obsolete: true
Attachment #8790637 - Flags: review+
Attachment #8790637 - Flags: feedback+
Pushed by ryanvm@gmail.com:
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/be466c64f9c0
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Blocks: 1302381
You need to log in before you can comment on or make changes to this bug.