Rewrite PointerlockFsWarning.Timeout to use ES6-class

RESOLVED FIXED in Firefox 53

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: xidorn, Assigned: Jalen Adams)

Tracking

({good-first-bug})

Trunk
Firefox 53
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

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

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

8 months ago
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)
(Assignee)

Comment 2

8 months ago
(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 hidden (mozreview-request)

Comment 5

8 months ago
mozreview-review
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)
(Reporter)

Comment 6

8 months ago
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)

Comment 7

8 months ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0c0de08391b4
Rewrite PointerlockFsWarning.Timeout to use ES6-class. r=jaws

Comment 8

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0c0de08391b4
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in before you can comment on or make changes to this bug.