Closed Bug 1082052 Opened 5 years ago Closed 5 years ago

[Messages][Refactoring] Return a Promise from SMIL.parse

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-master fixed)

RESOLVED FIXED
FxOS-S3 (24Jul)
Tracking Status
b2g-master --- fixed

People

(Reporter: julienw, Assigned: julienw, Mentored)

References

()

Details

(Whiteboard: [good first bug][lang=js][sms-FxOS-S3])

Attachments

(1 file)

SMIL.parse uses a callback to return its result. We should convert it to use a Promise instead.

The code is located in smil.js and is unit-tested, therefore it should be a good first bug for someone wanting to work on Gaia.
Hi Julienw
Here we have to use Window.promise instead of window.smil and other related to this in smil.js. Right?
Thanks
We have to return a promise instead of calling the callback, and change where it's called.

Not sure I understand your question though!
Note that another contributor asked me by mail to work on this. Rishav, since you start to be used to work on the SMS app, I think it's better if you let someone else take this "good first bug" :)
Yes, sure :)
Hey I'd like to work on this bug. First Gaia bug for me. How do i begin?
Hi anirudh
Hope you build the Gaia in your system, if not then follow this to build it first
https://developer.mozilla.org/en-US/Firefox_OS
Thanks
Hi Anirudh!

You can find some information about how to run the SMS app in Firefox in [1]. You'll also find information about unit tests in [2].

Please come on IRC (irc.mozilla.org, #gaia and #gaia-messaging) if you want to discuss directly!

[1] https://github.com/julienw/gaia/blob/1082618-sms-readme/apps/sms/README.md
[2] https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_unit_tests
(In reply to Julien Wajsberg [:julienw] from comment #7)
> Hi Anirudh!
> 
> You can find some information about how to run the SMS app in Firefox in
Hey I'm building gaia on my Linux PC. I'll get on with it as soon as its done building. Thanks!
Ok, adding you as assignee for now :)
Please report if you decide to not work on this again. Thanks!
Assignee: nobody → anirudh.gp
After generating the debug profile, when i run firefox -profile /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms, I get this:
(process:7318): GLib-CRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed
Firefox opens up with "chrome/branding/locale/browserconfig.properties" at the web address box
and the contents of the page are : "browser.startup.homepage=about:home"
Is this how its supposed to be have i done something wrong?
Flags: needinfo?(felash)
(In reply to anirudh.gp from comment #10)
> After generating the debug profile, when i run firefox -profile
> /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms, I get this:

You really need to launch firefox using:

firefox --no-remote -profile /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms app://sms.gaiamobile.org

> (process:7318): GLib-CRITICAL **: g_slice_set_config: assertion
> 'sys_page_size == 0' failed

The error appears for me too, I don't know what it means.

> Firefox opens up with "chrome/branding/locale/browserconfig.properties" at
> the web address box
> and the contents of the page are : "browser.startup.homepage=about:home"
> Is this how its supposed to be have i done something wrong?

This is strange, but please follow the directions :)
Flags: needinfo?(felash)
(In reply to Julien Wajsberg [:julienw] from comment #11)
> (In reply to anirudh.gp from comment #10)
> > After generating the debug profile, when i run firefox -profile
> > /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms, I get this:
> 
> You really need to launch firefox using:
> 
> firefox --no-remote -profile
> /home/gp/mozilla-source/firefox/mozilla-central/gaia/profile-sms
> app://sms.gaiamobile.org
> 
> > (process:7318): GLib-CRITICAL **: g_slice_set_config: assertion
> > 'sys_page_size == 0' failed
> 
> The error appears for me too, I don't know what it means.
> 
> > Firefox opens up with "chrome/branding/locale/browserconfig.properties" at
> > the web address box
> > and the contents ohpposed to be have i done something wrong?
> 
> This is strange, but please follow the directions :)
Thanks :) That worked!
T
Hey Anirudh, could you move forward here? :)
Flags: needinfo?(anirudh.gp)
I'm not well versed with javascript yet so i dont know how to convert it to return a promise. Can you help me?
Flags: needinfo?(anirudh.gp)
About Promises, I suggest to first read [1]. Please read this resource very thoroughly, and if you have questions after reading it, I'd suggest you come on IRC (irc.mozilla.org, channel #gaia or #gaia-messaging) and we can discuss.

[1] http://www.html5rocks.com/en/tutorials/es6/promises/
I could give a shot at this bug.
But I have a question. I see that parse might accept an array of attachments. You can especially see this in |attachments.forEach(SMIL_parseWithoutSMIL);| How would the promise workout here? Because for every blob processing it seems there is |exitPoint()| is called. So that I understand correctly exitPoint IS a returning spot from the function?
Flags: needinfo?(felash)
(In reply to Alexey Novak from comment #16)
> I could give a shot at this bug.
> But I have a question. I see that parse might accept an array of
> attachments. You can especially see this in
> |attachments.forEach(SMIL_parseWithoutSMIL);| How would the promise workout
> here? Because for every blob processing it seems there is |exitPoint()| is
> called. So that I understand correctly exitPoint IS a returning spot from
> the function?

There is an idea that replacing all the activeReaders related function into promise(readTextBlob/convertWbmpToPng), and replace exitPoint with Promise.all to wait for all the promises completed and return slides.
Flags: needinfo?(felash)
Summary: [Messages] Return a Promise from SMIL.parse → [Messages][Refactoring] Return a Promise from SMIL.parse
Reseting assignee.
Assignee: anirudh.gp → nobody
Actually taking it, I need it to make my life easier in bug 1162030.
Assignee: nobody → felash
Comment on attachment 8635336 [details] [review]
[gaia] julienw:1082052-smil-promises > mozilla-b2g:master

Hey Oleg, how does it look ?
Attachment #8635336 - Flags: review?(azasypkin)
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js][sms-FxOS-S3]
Blocks: 1162030
Comment on attachment 8635336 [details] [review]
[gaia] julienw:1082052-smil-promises > mozilla-b2g:master

Looks good, left some questions and nits at github.

Thanks!
Attachment #8635336 - Flags: review?(azasypkin) → review+
Blocks: 1185555
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/2f059a213fe84f114fc693b8e90b28c452dec5cb
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S3 (24Jul)
You need to log in before you can comment on or make changes to this bug.