Closed
Bug 1259656
Opened 8 years ago
Closed 8 years ago
Clean up WidgetEvent
Categories
(Core :: Widget, defect)
Core
Widget
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
Attachments
(7 files)
MozReview Request: Bug 1259656 part.1 Rename WidgetEvent::refPoint to WidgetEvent::mRefPoint r?smaug
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 |
See bug 1259654 comment 0 for the detail.
Assignee | ||
Updated•8 years ago
|
Mentor: masayuki
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → masayuki
Mentor: masayuki
Status: NEW → ASSIGNED
Whiteboard: [good first bug][for Gecko Inside #7 in Mozilla Japan]
Assignee | ||
Comment 1•8 years ago
|
||
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)
Assignee | ||
Comment 2•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0b4959bb72e
Comment 3•8 years ago
|
||
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.
Assignee | ||
Comment 4•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47369/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47369/
Attachment #8742631 -
Flags: review?(bugs)
Attachment #8742632 -
Flags: review?(bugs)
Attachment #8742633 -
Flags: review?(bugs)
Attachment #8742634 -
Flags: review?(bugs)
Attachment #8742635 -
Flags: review?(bugs)
Attachment #8742636 -
Flags: review?(bugs)
Attachment #8742637 -
Flags: review?(bugs)
Assignee | ||
Comment 5•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47371/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47371/
Assignee | ||
Comment 6•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47373/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47373/
Assignee | ||
Comment 7•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47375/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47375/
Assignee | ||
Comment 8•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47377/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47377/
Assignee | ||
Comment 9•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47379/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47379/
Assignee | ||
Comment 10•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47381/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47381/
Comment 11•8 years ago
|
||
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 12•8 years ago
|
||
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 13•8 years ago
|
||
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 14•8 years ago
|
||
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 15•8 years ago
|
||
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 16•8 years ago
|
||
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 17•8 years ago
|
||
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+
Updated•8 years ago
|
Flags: needinfo?(bugs)
Comment 18•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/327c61df0bae https://hg.mozilla.org/integration/mozilla-inbound/rev/dceb7f8f80d1 https://hg.mozilla.org/integration/mozilla-inbound/rev/79737c3fb41b https://hg.mozilla.org/integration/mozilla-inbound/rev/38b12742d84c https://hg.mozilla.org/integration/mozilla-inbound/rev/9fd7de6fc630 https://hg.mozilla.org/integration/mozilla-inbound/rev/f36d8bc4ac18 https://hg.mozilla.org/integration/mozilla-inbound/rev/a5f6f7406112
Comment 19•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/327c61df0bae https://hg.mozilla.org/mozilla-central/rev/dceb7f8f80d1 https://hg.mozilla.org/mozilla-central/rev/79737c3fb41b https://hg.mozilla.org/mozilla-central/rev/38b12742d84c https://hg.mozilla.org/mozilla-central/rev/9fd7de6fc630 https://hg.mozilla.org/mozilla-central/rev/f36d8bc4ac18 https://hg.mozilla.org/mozilla-central/rev/a5f6f7406112
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•