1235830 - Renaming GetInternalNSEvent to WidgetEvent, in nsIDOMEvent

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Aidin Gharibnavaz]

As Olli Pettay suggested in Bug 1230216 Comment 9

Comment 1

[Aidin Gharibnavaz]

"WidgetEvent" is also the name of a type, so I can't use it as the name of a method.
Should I change the name to "GetWidgetEvent"?

Comment 2

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

Oh, right. GetWidgetEvent() hints that it may return null, but it should never return null, so something else. Perhaps AsWidgetEvent() ? Not quite happy with that but nothing better comes to my mind now.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'm also using Get*() when * is a type/class name even if it never returns nullptr. Should we discuss this issue in dev-platform?

Comment 4

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

Perhaps, though I guess it has been mostly a dom/* convention that
Foo* GetFoo() may return null and Foo* AsFoo() or Foo* SomeOtherGetterListMethod() doesn't return null.

Comment 5

[Aidin Gharibnavaz]

Attachment: MozReview Request: Bug 1235830 - Renaming GetInternalNSEvent to WidgetEvent, in nsIDOMEvent r?smaug

Review commit: https://reviewboard.mozilla.org/r/34567/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34567/

Comment 6

[Aidin Gharibnavaz]

I asked on dev-platform, and Bobby Holley suggest the name WidgetEventRef. I think that's fine.

Here's the try server build without error:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4124741e66f4

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Why would we name it WidgetEventRef if it doesn't actually return a reference?

Comment 8

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

Yeah, WidgetEventRef isn't right here.
WidgetEventPtr() would be ok, or make the method to return WidgetEvent& and then use WidgetEventRef.

Comment 9

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

If WidgetEventPtr in .idl is causing trouble here, feel free to change that name.

Comment 10

[Aidin Gharibnavaz]

Comment on attachment 8718421 [details]
MozReview Request: Bug 1235830 - Renaming GetInternalNSEvent to WidgetEvent, in nsIDOMEvent r?smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34567/diff/1-2/

Comment 11

[Aidin Gharibnavaz]

Now it's look better (:

Here's the try server build:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1983d133e3dd

Comment 12

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

Comment on attachment 8718421 [details]
MozReview Request: Bug 1235830 - Renaming GetInternalNSEvent to WidgetEvent, in nsIDOMEvent r?smaug

https://reviewboard.mozilla.org/r/34567/#review31435

Ah, WidgetEvent is in mozilla namespace so .idl part should work.
Thanks.

Comment 13

[Aidin Gharibnavaz]

Thank you for the review (:

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/03fc467e1002

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/03fc467e1002