The default bug view has changed. See this FAQ.

Installation of web apps from Firefox Marketplace fails on Linux during mapping of categories

RESOLVED FIXED in 2013-07-25

Status

Marketplace
Consumer Pages
P2
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Eike Hein, Assigned: ngoke)

Tracking

({regression})

2013-07-25
x86_64
Linux
regression
Points:
---

Details

(Whiteboard: [qa+])

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
Created attachment 754286 [details] [diff] [review]
fix-webapps-install-on-linux.patch

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20130526 Firefox/24.0 (Nightly/Aurora)
Build ID: 20130526031046

Steps to reproduce:

I tried to install an application from Firefox Marketplace (http://marketplace.firefox.com).


Actual results:

Installation failed silently. The Error Console provides the following error:

  Error installing app: TypeError: (intermediate value).toLowerCase is not a function


Expected results:

The application should have finished successfully.

Now, I've looked at the relevant code (toolkit/webapps/WebappsInstaller.jsm) and debugged it a little, and it turns out that the reason for the failure is that it expects the list of application categories (this.app.categories) to be a list of strings, however Marketplace appears to hand it a list of integers. (At least I _think_ that the installPackage()[1] call is issued by JavaScript code served by the Marketplace web app - I'm super-new to this whole ecosystem and piecing things together by prodding ...)

The integers I'm seeing correspond to the numeric category id's available via the Marketplace API:https://marketplace.firefox.com/api/v1/apps/category/

So it seems Marketplace has changed behavior, intentionally or not, and the client code wasn't updated to match.

Since I'd like to ship a copy of Firefox with working Marketplace install in a product within a short timeframe, I've decided to work around this problem by way of the patch I've attached to this ticket. It extends the translation table from Marketplace categories to freedesktop.org categories by a set of numeric keys, and only attempts to call toLowerCase() on a category if typeof concludes that it's not a number.

This seemed to be the only way to go for me, since in case Marketplace decides to send down strings again as a result of this bug report -- the resulting code should handle both.


1 = https://developer.mozilla.org/en-US/docs/Web/API/Apps.installPackage?redirectlocale=en-US&redirectslug=DOM%2FApps.installPackage
(Reporter)

Updated

4 years ago
Component: Untriaged → Web Apps
This actually probably isn't the right place to do the workaround actually - I'd fix this on the marketplace side, not the client side code. The numbers carry no meaning in the client side code, but they do on the marketplace side.

In that argument, I'm bouncing this over to the Marketplace product.
Status: UNCONFIRMED → NEW
Component: Web Apps → Consumer Pages
Ever confirmed: true
Keywords: regression
Product: Firefox → Marketplace
Version: Trunk → 1.0
(Reporter)

Comment 2

4 years ago
Aye, personally I agree that it makes more sense to change Marketplace back(?) to using strings, considering clients expecting them are already out there now. But since I can do little about Marketplace locally, I dug into the client sources instead.
Sorry, since this jumped products I think I may have lost context.  I'm looking at the link from comment 0 (https://marketplace.firefox.com/api/v1/apps/category/) and I see an array of categories in this format:

> {"id": "155", "name": "Games", "resource_uri": "/api/v1/apps/category/155/", "slug": "games"}

Which gives you names, slugs, and IDs.  What are you asking for in this bug?
(In reply to Wil Clouser [:clouserw] from comment #3)
> Sorry, since this jumped products I think I may have lost context.  I'm
> looking at the link from comment 0
> (https://marketplace.firefox.com/api/v1/apps/category/) and I see an array
> of categories in this format:
> 
> > {"id": "155", "name": "Games", "resource_uri": "/api/v1/apps/category/155/", "slug": "games"}
> 
> Which gives you names, slugs, and IDs.  What are you asking for in this bug?

I think a while back marketplace implemented something where when you called the install function to install hosted app on linux, we would provide the category of the app as part of the dictionary in the second parameter (e.g. Games). The issue filed here is the fact that we're now passing the ID instead, not the category, so the client-side code is erroring out given that we're expecting a string in letters, not a number.
(Reporter)

Comment 5

4 years ago
That's correct.

And to perhaps drive this home a bit more, the consequence of this is that no Firefox release made since October 2012 (when a commit by Marco Castelluccio turned the reliance on strings into a runtime failure by introducing the toLowerCase() call) is capable of installing applications from Marketplace on Linux, and no version of Firefox ever released is capable of correctly associating Marketplace applications with menu categories on Linux (because the translation table from Marketplace categories to Linux menu categories is keyed by string).
It sounds like Kevin added this so assigning to him.
Assignee: nobody → ngoke
Priority: -- → P2
(Assignee)

Comment 7

4 years ago
https://github.com/mozilla/zamboni/commit/4398814875b6170f3ea0a0b95899c008ea7ba279

Not sure if this is correct, but when installing Wikipedia, it nows shows up under the "Other" category in Ubuntu. Before, it would not show up at all.
(Assignee)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2013-06-06
(Reporter)

Comment 8

4 years ago
That's fine, and common behavior among menu implementations which only show a defined set of top-level categories and treat the rest as "Other". Thanks!
(Reporter)

Comment 9

4 years ago
Has this been deployed already? Marketplace installation on Linux still fails with current Nightly (sans my patch).
(In reply to Eike Hein from comment #9)
> Has this been deployed already? Marketplace installation on Linux still
> fails with current Nightly (sans my patch).

Not yet. See the target milestone - this will be deployed on June 6th.
(Reporter)

Comment 11

4 years ago
Missed that metadata, thanks and sorry for the noise :).

Comment 12

4 years ago
hi,

I'm still getting this error using tonights nightly build for linux 64.

[23:39:00.157] Error installing app: TypeError: (intermediate value).toLowerCase is not a function @ resource://gre/modules/WebappsInstaller.jsm:40
[23:39:00.184] "[install] Error installing app: " "DENIED"

can I do something to help?
Duplicate of this bug: 879176
Has this been deployed? The bug is still valid.

Comment 15

4 years ago
Bug is still valid as of the nightly build of today
Flags: needinfo?(ngoke)
Flags: needinfo?(clouserw)
(Assignee)

Comment 16

4 years ago
I don't have access to a Linux box right now...but will later in the week. I could look at it then.
Flags: needinfo?(ngoke)

Comment 17

4 years ago
I'm happy to let people remote control my Linux machine for testing if that is useful?
(Assignee)

Comment 18

4 years ago
Thanks, but I guess I should just install a VM.
(Reporter)

Comment 19

4 years ago
FWIW, clients patched with my patch continue to work if any of you need a workaround.
Kevin, Marketplace is still passing an array of integers. This is why I asked if the patch had been deployed.
For example, for Wikipedia, it's passing [161,163].

Looking at your patch, if you're using this data (https://marketplace.firefox.com/api/v1/apps/category/) you should use cat.slug and not cat.id.

However Marketplace also changed the categories, in the future it would be good to keep them in sync with Firefox (for example "photos-media" has become "photo-video").
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 21

4 years ago
Marco: Ah okay, I was mistaken, I'll get on that right now.
Flags: needinfo?(clouserw)
(Assignee)

Updated

4 years ago
Target Milestone: 2013-06-06 → 2013-07-18
(Assignee)

Comment 22

4 years ago
Got a patch, wasn't too trivial. Should be in within the week.
(Assignee)

Comment 23

4 years ago
https://github.com/mozilla/zamboni/pull/891
(Assignee)

Comment 24

4 years ago
https://github.com/mozilla/zamboni/commit/65663ae4e4b62a3e7879bb6fb070d90c19291d98
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED

Comment 25

4 years ago
Amazing! It is working. Thanks very much for fixing it.
(Assignee)

Comment 26

4 years ago
Sweeettt
Whiteboard: [qa+]
Target Milestone: 2013-07-18 → 2013-07-25
(Reporter)

Comment 27

3 years ago
This has regressed again; neither Firefox 24 nor current nightly (28.0a1 (2013-10-29)) write any "Categories=" keys to the .desktop files.

This is obviously never going to work well this way. I suggest to move the category mapping server-side (perhaps even letting content authors edit it) and just write to the .desktop file whatever Marketplace pushes down to the client, removing complexity from the client.

Until then however a short-term fix would be much appreciated.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Was this related to bug 912279?
(Reporter)

Comment 29

3 years ago
The original bug was, yes - the server started returning integer IDs instead of strings, and so the baked-in mapping table was no longer useful for lookup, plus the code crashed trying to toLowerCase() an integer. I don't know if this is the same failure vector again, though.
This is a different bug. The installation isn't failing.
Filed bug 932994.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago3 years ago
Resolution: --- → FIXED
(Reporter)

Comment 31

3 years ago
Good point, thanks for the triage Marco.
(In reply to Eike Hein from comment #31)
> Good point, thanks for the triage Marco.

Thank you for noticing the problem!
You need to log in before you can comment on or make changes to this bug.