Closed Bug 1259656 Opened 4 years ago Closed 4 years ago

Clean up WidgetEvent

Categories

(Core :: Widget, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

Attachments

(7 files)

58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
58 bytes, text/x-review-board-request
smaug
: review+
Details
Assignee: nobody → masayuki
Mentor: masayuki
Status: NEW → ASSIGNED
Whiteboard: [good first bug][for Gecko Inside #7 in Mozilla Japan]
Smaug:

I've needed log time to understand what's the purpose of userType. I think that it should be renamed something like WidgetEvent::mSpecifiedEventType because it's used when:
1. The event is WidgetCommandEvent (stores the command)
2. The event type specified is unknown event type
3. The event type is known, but for different event interface (e.g., "keydown" for "MouseEvent")

Do you have better idea?
Flags: needinfo?(bugs)
WidgetCommandEvent doesn't store the command there. It is the event type. It is just that appcommand doesn't have any specific eFooEvent value.

So, (2) and (3) apply here.
mSpecifiedEventType is ok-ish. At least don't have better ideas for the name atm.
Comment on attachment 8742631 [details]
MozReview Request: Bug 1259656 part.1 Rename WidgetEvent::refPoint to WidgetEvent::mRefPoint r?smaug

https://reviewboard.mozilla.org/r/47369/#review44241
Attachment #8742631 - Flags: review?(bugs) → review+
Comment on attachment 8742632 [details]
MozReview Request: Bug 1259656 part.2 Rename WidgetEvent::lastRefPoint to WidgetEvent::mLastRefPoint r?smaug

https://reviewboard.mozilla.org/r/47371/#review44245
Attachment #8742632 - Flags: review?(bugs) → review+
Comment on attachment 8742633 [details]
MozReview Request: Bug 1259656 part.3 Rename WidgetEvent::userType to WidgetEvent::mSpecifiedEventType r?smaug

https://reviewboard.mozilla.org/r/47373/#review44251
Attachment #8742633 - Flags: review?(bugs) → review+
Comment on attachment 8742634 [details]
MozReview Request: Bug 1259656 part.4 Rename WidgetEvent::typeString to WidgetEvent::mSpecifiedEventTypeString r?smaug

https://reviewboard.mozilla.org/r/47375/#review44253

::: widget/BasicEvents.h:362
(Diff revision 1)
>      mRefPoint = aEvent.mRefPoint;
>      // mLastRefPoint doesn't need to be copied.
>      AssignEventTime(aEvent);
>      // mFlags should be copied manually if it's necessary.
>      mSpecifiedEventType = aEvent.mSpecifiedEventType;
> -    // typeString should be copied manually if it's necessary.
> +    // mSpecifiedEventTypeString should be copied manually if it's necessary.

Not about this bug, but I wonder actually why it should be copied if needed, and why not always.
Feels error prone.
Attachment #8742634 - Flags: review?(bugs) → review+
Comment on attachment 8742635 [details]
MozReview Request: Bug 1259656 part.5 Rename WidgetEvent::target to WidgetEvent::mTarget r?smaug

https://reviewboard.mozilla.org/r/47377/#review44257
Attachment #8742635 - Flags: review?(bugs) → review+
Comment on attachment 8742636 [details]
MozReview Request: Bug 1259656 part.6 Rename WidgetEvent::currentTarget to WidgetEvent::mCurrentTarget r?smaug

https://reviewboard.mozilla.org/r/47379/#review44259
Attachment #8742636 - Flags: review?(bugs) → review+
Comment on attachment 8742637 [details]
MozReview Request: Bug 1259656 part.7 Rename WidgetEvent::originalTarget to WidgetEvent::mOriginalTarget r?smaug

https://reviewboard.mozilla.org/r/47381/#review44263
Attachment #8742637 - Flags: review?(bugs) → review+
Flags: needinfo?(bugs)
You need to log in before you can comment on or make changes to this bug.