Closed
Bug 755044
Opened 12 years ago
Closed 6 years ago
App installs fail if "installs_allowed_from" field value has trailing slashes
Categories
(Core Graveyard :: DOM: Apps, defect)
Core Graveyard
DOM: Apps
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: krupa.mozbugs, Assigned: fabrice)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
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.
Reporter | ||
Comment 1•12 years ago
|
||
For now, we are applying a band-aid in Marketplace by implementing bug 755047
Updated•12 years ago
|
Component: Web Apps → DOM: Mozilla Extensions
Product: Firefox → Core
QA Contact: webapps → general
Comment 2•12 years ago
|
||
Sounds like a mozapps API bug. Fabrice and Ian - Can you confirm this to be a mozapps API bug?
Updated•12 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 3•12 years ago
|
||
Yes, this seems like a mozApps bug, the values should be normalized when checking for matches.
Assignee | ||
Comment 4•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
(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•12 years ago
|
||
Can someone take point on fixing this bug?
Updated•12 years ago
|
blocking-kilimanjaro: --- → ?
Updated•12 years ago
|
tracking-firefox15:
--- → ?
Updated•12 years ago
|
blocking-kilimanjaro: ? → ---
Updated•12 years ago
|
Comment 8•12 years ago
|
||
Assigning to bz for now, as a module peer, to hopefully find someone to take the lead on deeper investigation of this issue.
Assignee: nobody → bzbarsky
Updated•12 years ago
|
tracking-firefox15:
+ → ---
tracking-firefox16:
--- → ?
Comment 9•12 years ago
|
||
Desktop Apps is now targeting FF 16 - switching the flag to say as such.
Comment 10•12 years ago
|
||
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•12 years ago
|
||
Or are we just using this component as a dumping ground for every API someone decides to expose to web pages, basically?
(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•12 years ago
|
||
(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•12 years ago
|
||
(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•12 years ago
|
||
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•12 years ago
|
||
(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•12 years ago
|
||
(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•12 years ago
|
||
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.
Updated•12 years ago
|
tracking-firefox17:
--- → +
Comment 19•12 years ago
|
||
Anant - What should we do here in this use case?
Comment 20•12 years ago
|
||
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•12 years ago
|
||
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.
Assignee: bzbarsky → anant
Comment 22•12 years ago
|
||
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.
Assignee: anant → nobody
blocking-basecamp: --- → ?
tracking-firefox16:
+ → ---
tracking-firefox17:
+ → ---
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → fabrice
Comment 23•12 years ago
|
||
Basecamp-. We should add something to the dev docs to indicate that we won't support this.
Updated•12 years ago
|
Assignee: nobody → fabrice
Updated•12 years ago
|
Blocks: Apps-Dev-Doc-Needed
Comment 24•12 years ago
|
||
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
Keywords: dev-doc-needed → dev-doc-complete
Updated•7 years ago
|
Product: Core → Core Graveyard
Comment 25•6 years ago
|
||
Core Graveyard / DOM: Apps is inactive. Closing all bugs in this component.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•