Use cloneInto instead of exposedProps in b2g/chrome/content/shell.js

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Because it'll simplify code and be a little faster/easier to think about / deal with.
(Assignee)

Comment 1

4 years ago
Created attachment 8439274 [details] [diff] [review]
use cloneInto instead of exposedProps in b2g's shell.js,

AFAICT this should work just fine. sendCustomEvent already does the Cu.cloneInto dance for us, so we can just use that instead. It also uses the content window, so even more code can be removed. As for the special-casing of the screenshot stuff, that was added in 2012 in bug 784205. Since then, cloneInto has improved, as far as I can tell - at least, a quick test on jsbin in my desktop browser shows that a Blob seems to get dealt with fine. Try push: https://tbpl.mozilla.org/?tree=Try&rev=0bf9bbdfe090
(Assignee)

Comment 2

4 years ago
Comment on attachment 8439274 [details] [diff] [review]
use cloneInto instead of exposedProps in b2g's shell.js,

Alex, can you review this? Fabrice seems to be out and I don't know who else to ask, looking at hg log. The FxOS modules page on wikimo wasn't helpful (I'm not even sure to which submodule this belongs, as most of them don't have paths...). :-\
Attachment #8439274 - Flags: review?(poirot.alex)
Comment on attachment 8439274 [details] [diff] [review]
use cloneInto instead of exposedProps in b2g's shell.js,

Review of attachment 8439274 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me and works fine on my device (I was especially suspicious about the screenshot Blob).

This code belongs to b2g/ folder whose official peers are Vivien, Fabrice and Chris:
  https://wiki.mozilla.org/Modules/All#FirefoxOS
Attachment #8439274 - Flags: review?(poirot.alex) → review+
(Assignee)

Comment 4

4 years ago
With both Vivien and Fabrice on PTO and Chris gone, is this r+ enough and can I just land this, or do I need to ask someone else?
Flags: needinfo?(poirot.alex)
I got offline approval from Vivien.
Flags: needinfo?(poirot.alex)
(Assignee)

Comment 6

4 years ago
remote:   https://hg.mozilla.org/integration/b2g-inbound/rev/cff987e7f540
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/cff987e7f540
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.