Last Comment Bug 755044 - App installs fail if "installs_allowed_from" field value has trailing slashes
: App installs fail if "installs_allowed_from" field value has trailing slashes
Status: NEW
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Apps (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: ---
Assigned To: [:fabrice] Fabrice Desré
:
: [:fabrice] Fabrice Desré
Mentors:
Depends on:
Blocks: Apps-Dev-Doc-Needed
  Show dependency treegraph
 
Reported: 2012-05-14 14:31 PDT by krupa raj[:krupa]
Modified: 2013-06-02 09:23 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description krupa raj[:krupa] 2012-05-14 14:31:52 PDT
steps to reproduce:
1. Load https://marketplace.mozilla.org/en-US/app/qr-code-generator/
2. Try to install the app

observed behavior:
Firefox fails to install the app if "installs_allowed_from" field value has trailing slashes.i.e.,

"installs_allowed_from": [
   "https://marketplace.mozilla.org"
 ],
is correct but
 "installs_allowed_from": [
   "https://marketplace.mozilla.org/"
 ],
is not.

Marketplace however allows trailing slashes and Firefox should too.

Note: Until we fix this in Firefox, Marketplace will temporarily stop allowing this.
Comment 1 krupa raj[:krupa] 2012-05-14 14:35:39 PDT
For now, we are applying a band-aid in Marketplace by implementing bug 755047
Comment 2 Jason Smith [:jsmith] 2012-05-14 15:24:10 PDT
Sounds like a mozapps API bug. Fabrice and Ian - Can you confirm this to be a mozapps API bug?
Comment 3 Ian Bicking (:ianb) 2012-05-14 15:26:09 PDT
Yes, this seems like a mozApps bug, the values should be normalized when checking for matches.
Comment 4 [:fabrice] Fabrice Desré 2012-05-14 15:41:26 PDT
if we consider that installs_allowed_from contains origins (and I think it should), "https://marketplace.mozilla.org/" is invalid. In such cases, should we normalize them to their origin, or discard them?
Comment 5 Ian Bicking (:ianb) 2012-05-14 15:44:03 PDT
Explaining the subtle distinction between https://marketplace.mozilla.org/ and https://marketplace.mozilla.org seems like more work and less developer-friendly than just handling this case.  Also I believe we aren't normalizing with respect to case and ports currently, and HTTP://MARKETPLACE.MOZILLA.ORG is a valid origin.  So some normalization is in order.
Comment 6 Wil Clouser [:clouserw] 2012-05-16 14:47:22 PDT
(In reply to krupa raj 82[:krupa] from comment #1)
> For now, we are applying a band-aid in Marketplace by implementing bug 755047

I'd like to see an ETA in this bug before we start talking about temporary patches on AMO.
Comment 7 Jason Smith [:jsmith] 2012-05-16 14:48:19 PDT
Can someone take point on fixing this bug?
Comment 8 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-18 15:51:03 PDT
Assigning to bz for now, as a module peer, to hopefully find someone to take the lead on deeper investigation of this issue.
Comment 9 Jason Smith [:jsmith] 2012-07-18 15:56:24 PDT
Desktop Apps is now targeting FF 16 - switching the flag to say as such.
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2012-07-18 20:18:23 PDT
I have no idea what this bug is talking about... it's not part of the "DOM" as I know it.  Someone care to explain?
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2012-07-18 20:19:04 PDT
Or are we just using this component as a dumping ground for every API someone decides to expose to web pages, basically?
Comment 12 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-18 20:19:48 PDT
(In reply to Boris Zbarsky (:bz) from comment #11)
> Or are we just using this component as a dumping ground for every API
> someone decides to expose to web pages, basically?

Yep!
Comment 13 Jason Smith [:jsmith] 2012-07-18 23:37:00 PDT
(In reply to Boris Zbarsky (:bz) from comment #10)
> I have no idea what this bug is talking about... it's not part of the "DOM"
> as I know it.  Someone care to explain?

It's a mozapps API bug. In the app manifest (https://developer.mozilla.org/en/Apps/Manifest), if you specify trailing slashes on an installs_allowed_from value, then you won't be able to install the app with that manifest.

In regards to who typically handles these bugs - usually it's developers who have worked with the mozapps API on the apps team.
Comment 14 Jason Smith [:jsmith] 2012-07-18 23:38:05 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #12)
> (In reply to Boris Zbarsky (:bz) from comment #11)
> > Or are we just using this component as a dumping ground for every API
> > someone decides to expose to web pages, basically?
> 
> Yep!

Sorry about that. I'll followup on the bug to migrate the webapps stuff to a core component (there's a bug on file to take care of this, but traction on it has been slow).
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2012-07-18 23:52:15 PDT
Jason, thanks for the documentation link!

David, any idea who should own this?  You seem to have at least written tests for this stuff in the past.... ;)
Comment 16 Jason Smith [:jsmith] 2012-07-18 23:56:50 PDT
(In reply to Boris Zbarsky (:bz) from comment #15)
> Jason, thanks for the documentation link!
> 
> David, any idea who should own this?  You seem to have at least written
> tests for this stuff in the past.... ;)

Bill Walker and Myk (cc-ed here) would be good people to ask (both of them drive the apps engineering meetings weekly). Bill or Myk - What's your thoughts on this bug?
Comment 17 Olli Pettay [:smaug] 2012-07-19 00:26:14 PDT
(In reply to Boris Zbarsky (:bz) from comment #11)
> Or are we just using this component as a dumping ground for every API
> someone decides to expose to web pages, basically?

There is Bug 766386.
Comment 18 Myk Melez [:myk] [@mykmelez] 2012-07-23 14:28:52 PDT
Anant and Jonas are in the W3C working group standardizing the webapps DOM API, so the best owner would be one of them.  cc:ing them.
Comment 19 Jason Smith [:jsmith] 2012-07-30 10:30:22 PDT
Anant - What should we do here in this use case?
Comment 20 Anant Narayanan [:anant] 2012-07-30 11:26:08 PDT
In the spirit of HTML and many other WebAPIs it makes most sense to be lenient about how much we enforce the correctness of an origin. While the presence of a trailing slash is technically incorrect, it is well worth the effort to tolerate it.

However, we should definitely normalize as suggest by Ian, and always store valid origins in any of our code; which includes Firefox, the various WebRTs and the Marketplace (in this particular case, the trailing slash should be stripped before it is stored).
Comment 21 Boris Zbarsky [:bz] (still a bit busy) 2012-07-30 21:01:10 PDT
I'm still totally the wrong owner for this.  Picking a victim based on comment 18, with the assumption that he'll reassign as needed.
Comment 22 Jason Smith [:jsmith] 2012-07-31 16:56:16 PDT
Based on the DOM: Apps triage strategy specification, a bug needs to be considered a blocker for basecamp for resources to be allocated to it (to keep the focus on priorities). Based on that, I'm removing the tracking flags given that won't drive whether we decide to fix this in the short-term (I'm unassigning as well).

Given that was in question in the triage, I'm putting this in the nom queue for basecamp triage analysis.
Comment 23 Jason Smith [:jsmith] 2012-08-03 10:32:51 PDT
Basecamp-. We should add something to the dev docs to indicate that we won't support this.
Comment 24 Mark Giffin [:markg] 2012-09-19 15:03:26 PDT
I added a note about not using trailing slashes in installs_allowed_from in the main MDN page about manifests here:

https://developer.mozilla.org/en-US/docs/Apps/Manifest

Note You need to log in before you can comment on or make changes to this bug.