Closed
Bug 965023
Opened 11 years ago
Closed 11 years ago
Use Object.freeze on exports of Home.jsm
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 30
People
(Reporter: mcomella, Assigned: lpy)
References
Details
(Whiteboard: [mentor=mcomella][lang=js])
Attachments
(1 file, 2 obsolete files)
|
(deleted),
patch
|
mcomella
:
review+
|
Details | Diff | Splinter Review |
Like HomeProvider.jsm.
|
Reporter | |
Updated•11 years ago
|
Whiteboard: [mentor=mcomella][lang=js]
|
Assignee | |
Comment 1•11 years ago
|
||
Assignee: nobody → pylaurent1314
Attachment #8370764 -
Flags: review?(michael.l.comella)
|
|
Reporter | |
Comment 2•11 years ago
|
||
Comment on attachment 8370764 [details] [diff] [review]
bug965023.patch
Review of attachment 8370764 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/modules/Home.jsm
@@ +238,2 @@
> banner: HomeBanner,
> panels: HomePanels,
The `HomeBanner` and `HomePanels` objects, and some of their attributes are exported as well so these also need to be frozen.
Don't bother freezing private members - these will be hidden in bug 968378 (you're welcome to work on that one after this too! ;)
Attachment #8370764 -
Flags: review?(michael.l.comella) → review-
|
||
Comment 3•11 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #2)
> Comment on attachment 8370764 [details] [diff] [review]
> bug965023.patch
>
> Review of attachment 8370764 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: mobile/android/modules/Home.jsm
> @@ +238,2 @@
> > banner: HomeBanner,
> > panels: HomePanels,
>
> The `HomeBanner` and `HomePanels` objects, and some of their attributes are
> exported as well so these also need to be frozen.
Doesn't Object.freeze() freeze all members recursively?
|
|
Assignee | |
Comment 4•11 years ago
|
||
Attachment #8370764 -
Attachment is obsolete: true
Attachment #8371413 -
Flags: review?(michael.l.comella)
|
|
Reporter | |
Comment 5•11 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #3)
> Doesn't Object.freeze() freeze all members recursively?
The documentation [1] mentions that the freeze is shallow.
[1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze
Status: NEW → ASSIGNED
|
|
Reporter | |
Comment 6•11 years ago
|
||
Comment on attachment 8371413 [details] [diff] [review]
bug965023-V2.patch
Review of attachment 8371413 [details] [diff] [review]:
-----------------------------------------------------------------
One more level! I can still append to `Home.panels.Layout`. You can test this code with an addon I've been working on [1]. You can get the output with `adb logcat -v time | grep GeckoConsole`.
[1]: https://github.com/mcomella/fennec_rss/tree/obj_freeze
Attachment #8371413 -
Flags: review?(michael.l.comella) → review-
|
|
Assignee | |
Comment 7•11 years ago
|
||
Attachment #8371413 -
Attachment is obsolete: true
Attachment #8372089 -
Flags: review?(michael.l.comella)
|
|
Assignee | |
Comment 8•11 years ago
|
||
Thank you
|
|
Reporter | |
Comment 9•11 years ago
|
||
Comment on attachment 8372089 [details] [diff] [review]
bug965023-V3.patch
Review of attachment 8372089 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me! :)
Attachment #8372089 -
Flags: review?(michael.l.comella) → review+
|
|
Assignee | |
Updated•11 years ago
|
Keywords: checkin-needed
|
||
Comment 10•11 years ago
|
||
Keywords: checkin-needed
Whiteboard: [mentor=mcomella][lang=js] → [mentor=mcomella][lang=js][fixed-in-fx-team]
|
||
Comment 11•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [mentor=mcomella][lang=js][fixed-in-fx-team] → [mentor=mcomella][lang=js]
Target Milestone: --- → Firefox 30
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•