Closed
Bug 957418
Opened 11 years ago
Closed 11 years ago
Enable Inter-App Communication API for non-release builds of Firefox
Categories
(Core :: DOM: Device Interfaces, defect)
Core
DOM: Device Interfaces
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: kgrandon, Assigned: kgrandon)
References
Details
Attachments
(2 files)
FirefoxOS developers would like to be able to develop features which depend on this API in the browser. We can turn this on by default everywhere, but it seems like a safer option, and one that would be easier to get support for, would be to enable it for only non-release builds of Firefox for now.
Assignee | ||
Comment 1•11 years ago
|
||
Gecko Patch
Assignee | ||
Updated•11 years ago
|
Attachment #8356914 -
Attachment description: 0001-Bug-957418-Enable-Inter-App-Communication-API-for-no.patch → Gecko Patch - Enable Inter-App API for non-release builds
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 8356914 [details] [diff] [review]
Gecko Patch - Enable Inter-App API for non-release builds
Hi Ben -
Any chance you'd be able to review this one? Jonas is on PTO for a while, and we want to move quickly on these features. Thanks!
Attachment #8356914 -
Flags: review?(bent.mozilla)
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8356916 [details] [review]
Gaia Patch - Enable Inter-App API Pref
Alex, or Yuren - Could one of you guys review the simple build changes here? (Adding a pref). Probably shouldn't land this until the gecko patch does. Thanks!
Attachment #8356916 -
Flags: review?(yurenju.mozilla)
Attachment #8356916 -
Flags: review?(poirot.alex)
Comment 5•11 years ago
|
||
Comment on attachment 8356916 [details] [review]
Gaia Patch - Enable Inter-App API Pref
Hi Kevin, it would be better if generate |rules| on build time to support different domain name and port if 3rd party developer change them.
Attachment #8356916 -
Flags: review?(yurenju.mozilla)
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Yuren Ju [:yurenju] from comment #5)
> Comment on attachment 8356916 [details] [review]
> Gaia Patch - Enable Inter-App API Pref
>
> Hi Kevin, it would be better if generate |rules| on build time to support
> different domain name and port if 3rd party developer change them.
I disagree that 3rd party developers will be changing anything. I do think that a build script to inject additional hostnames into these manifests may be a better approach for the time being. I'm not sure if I have time to do that, but I'll investigate.
Assignee | ||
Comment 7•11 years ago
|
||
Comment on attachment 8356916 [details] [review]
Gaia Patch - Enable Inter-App API Pref
Hi Yuren - What do you think about something like this?
Attachment #8356916 -
Flags: review?(yurenju.mozilla)
Comment 8•11 years ago
|
||
Hi Kevin,
I'm still wondering why we write the manifestURLs in manifest.webapp, it should be generated in build time or we will have some problems if GAIA_DOMAIN is changed and hard to find the root cause.
we can have a Makefile in apps/search and generate manifest.webapp by a manifest.template, you can reference Makefile in apps/email to create a build script in apps/search/build/ for generating manifest.webapp.
does it make sense?
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to Yuren Ju [:yurenju] from comment #8)
> Hi Kevin,
>
> I'm still wondering why we write the manifestURLs in manifest.webapp, it
> should be generated in build time or we will have some problems if
> GAIA_DOMAIN is changed and hard to find the root cause.
>
> we can have a Makefile in apps/search and generate manifest.webapp by a
> manifest.template, you can reference Makefile in apps/email to create a
> build script in apps/search/build/ for generating manifest.webapp.
>
> does it make sense?
Do we have a use case where gaia domain is actually changed? I also wanted the ability to have a more generic build script that has the ability to fix multiple apps, instead of special casing this only for the search app. We will run into this problem everywhere we use the interapp API for now.
Comment on attachment 8356914 [details] [diff] [review]
Gecko Patch - Enable Inter-App API for non-release builds
I think this is more of a question for ehsan.
Attachment #8356914 -
Flags: review?(bent.mozilla) → review?(ehsan)
Comment 11•11 years ago
|
||
Comment on attachment 8356914 [details] [diff] [review]
Gecko Patch - Enable Inter-App API for non-release builds
Review of attachment 8356914 [details] [diff] [review]:
-----------------------------------------------------------------
Note that the API itself is hidden behind the dom.inter-app-communication-api.enabled pref. This patch just adds the missing JS modules to the packaging. The pref remains disabled on non-b2g environments.
Attachment #8356914 -
Flags: review?(ehsan) → review+
Comment 13•11 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #9)
> Do we have a use case where gaia domain is actually changed? I also wanted
> the ability to have a more generic build script that has the ability to fix
> multiple apps, instead of special casing this only for the search app. We
> will run into this problem everywhere we use the interapp API for now.
We certainly don't want "gaiamobile.org" becoming a fixture of the web, just like the value "Mozilla" in |navigator.appCodeName|
http://www.w3.org/html/wg/drafts/html/master/webappapis.html#the-navigator-object
If we deny the capability for venders to customize this value in the future, every FxOS device out there will be using |gaiamobile.org| for eternity.
Actually, IMHO, we should generate a set of UUIDs for our apps instead of keep using these made-up hostnames. But this is not urgent in anyway.
Assignee | ||
Comment 14•11 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #13)
> (In reply to Kevin Grandon :kgrandon from comment #9)
> > Do we have a use case where gaia domain is actually changed? I also wanted
> > the ability to have a more generic build script that has the ability to fix
> > multiple apps, instead of special casing this only for the search app. We
> > will run into this problem everywhere we use the interapp API for now.
>
> We certainly don't want "gaiamobile.org" becoming a fixture of the web, just
> like the value "Mozilla" in |navigator.appCodeName|
>
> http://www.w3.org/html/wg/drafts/html/master/webappapis.html#the-navigator-
> object
>
> If we deny the capability for venders to customize this value in the future,
> every FxOS device out there will be using |gaiamobile.org| for eternity.
>
> Actually, IMHO, we should generate a set of UUIDs for our apps instead of
> keep using these made-up hostnames. But this is not urgent in anyway.
I totally agree that GAIA_DOMAIN should be customizable, and should work everywhere. This patch actually helps that effort.
Regarding the manifest issue: I am just following the WebAPI implementation, and do not think that checking in a manifest.template for several apps is the proper solution.
Comment 15•11 years ago
|
||
Comment on attachment 8356916 [details] [review]
Gaia Patch - Enable Inter-App API Pref
Kevin is right, this pull request can handle different GAIA_DOMAIN. I left some comments on github, please fix nits before land it, thank you.
Attachment #8356916 -
Flags: review?(yurenju.mozilla) → review+
Assignee | ||
Updated•11 years ago
|
Attachment #8356916 -
Flags: review?(poirot.alex)
Assignee | ||
Comment 16•11 years ago
|
||
Adding checkin-needed for the gecko patch here: https://bug957418.bugzilla.mozilla.org/attachment.cgi?id=8356914
Keywords: checkin-needed
Assignee | ||
Comment 17•11 years ago
|
||
Thank you very much for the review Yuren. Gaia patch landed: https://github.com/mozilla-b2g/gaia/commit/f08d39eeead2c86712bbaa5ce8cbcadeeb103c7a
Assignee | ||
Comment 18•11 years ago
|
||
Re-landed gaia patch due to invalid push: https://github.com/mozilla-b2g/gaia/commit/87f6166cd68c8296cd9a183f9337c90046347b60
Comment 19•11 years ago
|
||
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment 21•11 years ago
|
||
Kevin, do we still need |manifestInterAppHostnames()| in webapp-manifests.js? seems we no longer need it.
Flags: needinfo?(kgrandon)
Assignee | ||
Comment 22•11 years ago
|
||
(In reply to Yuren [:yurenju][:小朱] from comment #21)
> Kevin, do we still need |manifestInterAppHostnames()| in
> webapp-manifests.js? seems we no longer need it.
Yes, we actually still need this to fix the origin case when running in Firefox Nightly. We change the origin from app:// to http:// so the ports will work properly in Nightly, so we still need this unless there is a better way of doing it.
Flags: needinfo?(kgrandon)
Comment 23•11 years ago
|
||
got it, thanks.
You need to log in
before you can comment on or make changes to this bug.
Description
•