Closed Bug 1326581 Opened 7 years ago Closed 7 years ago

Rewrite PointerlockFsWarning.Timeout to use ES6-class

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: xidorn, Assigned: julianne)

Details

(Keywords: good-first-bug, Whiteboard: [good first bug][lang=js])

Attachments

(1 file)

PointerlockFsWarning.Timeout [1] was initially implemented as a class [2], but at that moment es6-class was not enabled on any channel other than Nightly, so it was eventually changed to use the tranditional manner.

The traditional manner causes some issue when rewriting functions to use object-shorthand rule in bug 1325464. Given that we've had es6-class everywhere now, we can use that to make it clearer (and remove exception for eslint).


[1] https://dxr.mozilla.org/mozilla-central/rev/81463aec62d02fa1915e03cda7a8ddc8d44c71fc/browser/base/content/browser-fullScreenAndPointerLock.js#12-30,45-49
[2] https://reviewboard.mozilla.org/r/14841/diff/3/
Jalen, would you like to work on this? Might be a cool opportunity to pick up some more experience with new JavaScript language features.
Flags: needinfo?(leftysolara)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> Jalen, would you like to work on this? Might be a cool opportunity to pick
> up some more experience with new JavaScript language features.

I can work on this one. We just need to have Timeout changed back to the way it was in [2], correct?
Flags: needinfo?(leftysolara) → needinfo?(jaws)
Yeah, that looks right.
Assignee: nobody → leftysolara
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
Comment on attachment 8824266 [details]
Bug 1326581 - Rewrite PointerlockFsWarning.Timeout to use ES6-class.

https://reviewboard.mozilla.org/r/102800/#review103484

Looks good, I'm not sure I understand why the Timeout prototype was needed to be delcared within a PointerlockFsWarning.init() function. :xidorn, is there something I'm missing or does this change look OK?
Attachment #8824266 - Flags: review?(jaws) → review+
Flags: needinfo?(xidorn+moz)
I don't think there is any particular reason I have to put it in an init function. Probably we can also initialize Timeout.prototype in top level. I probably just wanted to avoid putting code there.
Flags: needinfo?(xidorn+moz)
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c0de08391b4
Rewrite PointerlockFsWarning.Timeout to use ES6-class. r=jaws
https://hg.mozilla.org/mozilla-central/rev/0c0de08391b4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: