Open
Bug 1295098
Opened 8 years ago
Updated 2 years ago
WidgetMouseEventBase::buttonType should be an enum class in mozilla::
Categories
(Core :: DOM: UI Events & Focus Handling, defect, P3)
Core
DOM: UI Events & Focus Handling
Tracking
()
NEW
Tracking | Status | |
---|---|---|
firefox51 | --- | affected |
People
(Reporter: masayuki, Unassigned)
References
(Blocks 1 open bug)
Details
WidgetMouseEventBase::buttonType is too long to use it in our coding rules (80 characters per line rule). And now, we can use enum class.
So, I think that we can redefine it as enum class for type safer like:
namespace mozilla {
typdef uint8 MouseButtonType;
enum class MouseButton
{
eLeft,
eMiddle,
eRight,
e4th,
e5th,
eUnknown
};
} // namespace mozilla
in mozilla/EventForwards.h
Additionally, we should make WidgetMouseEventBase::button a private member (but dom::MouseEvent class should be a friend class to access the raw value because JS can set its value to any value) and Create |MouseButton Button() const| which casts from integer to MouseButton.
Then, any default event handlers in C++ can check the button value safer.
Smaug, I'd like to use this bug in a hacker event in Mozilla Japan at the end of this month. Do you have any suggestion or objection for this bug?
Flags: needinfo?(bugs)
Comment 1•8 years ago
|
||
Hmm, how would this work in case JS initiates mouse button, say with number 100 ?
Would we need to everywhere explicitly check whether casting type back to MouseButton is safe?
Flags: needinfo?(bugs)
Reporter | ||
Comment 2•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #1)
> Hmm, how would this work in case JS initiates mouse button, say with number
> 100 ?
> Would we need to everywhere explicitly check whether casting type back to
> MouseButton is safe?
No, I think that if the raw value isn't 0-4, WidgetMouseButtonBase::Button() should return MouseButton::eUnknown. When a default event handler of MouseEvent handles new button, the author should define new button with the enum class. (I assume that nobody handles unknown button in default event handlers.)
Comment 3•8 years ago
|
||
But what would MouseEvent.button return in JS then?
Reporter | ||
Comment 4•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #3)
> But what would MouseEvent.button return in JS then?
Then, dom::MouseEvent just returns its raw value which is stored in WidgetMouseEvent::button (int16_t).
Updated•8 years ago
|
Priority: -- → P2
Comment 5•8 years ago
|
||
Oh, I see. C++ callers would use MouseButton Button() const method, which hides all the casting.
Ok, sounds reasonable.
Reporter | ||
Comment 6•8 years ago
|
||
Exactly. Thank you for your time.
Comment 7•8 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) from comment #0)
> Smaug, I'd like to use this bug in a hacker event in Mozilla Japan at the
> end of this month. Do you have any suggestion or objection for this bug?
Did the event happen? If so, I guess this bug didn't get included :)
Flags: needinfo?(masayuki)
Updated•8 years ago
|
Priority: P2 → P3
Reporter | ||
Comment 8•8 years ago
|
||
Yeah, this required a lot of code change than as I expected. I'll take this when I have much time.
Flags: needinfo?(masayuki)
Assignee | ||
Updated•6 years ago
|
Component: Event Handling → User events and focus handling
Updated•4 years ago
|
Summary: WidgetMosueEventBase::buttonType should be an enum class in mozilla:: → WidgetMouseEventBase::buttonType should be an enum class in mozilla::
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•