Last Comment Bug 746645 - Adding the white-list for myapps (or stage-myapps) has no effect with a clean FF profile
: Adding the white-list for myapps (or stage-myapps) has no effect with a clean...
Status: VERIFIED FIXED
: regression
Product: Firefox Graveyard
Classification: Graveyard
Component: Web Apps (show other bugs)
: 14 Branch
: All All
: -- critical
: Firefox 14
Assigned To: Ian Bicking (:ianb)
: Jason Smith [:jsmith]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-18 10:33 PDT by JStagner@Mozilla.com
Modified: 2016-02-04 15:00 PST (History)
11 users (show)
jsmith: in‑moztrap-
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix regression (1.04 KB, patch)
2012-04-19 12:22 PDT, Ian Bicking (:ianb)
fabrice: review+
mark.finkle: approval‑mozilla‑central+
Details | Diff | Splinter Review

Description JStagner@Mozilla.com 2012-04-18 10:33:18 PDT
Navigating to https://myapps.mozillalabs.com displays a screen to add a preference dom.mozApps.whitelist

On machines where I added the white list in a prior nightly and have updated to today's nightly - this works. On NEW installs (or complete re-installs) of nightly adding the whitelist seems to have n effect. 

Tested on Windows 7 and Mac OSX
Comment 1 Jason Smith [:jsmith] 2012-04-18 10:35:26 PDT
May be related to bug 746629. Sounds like a FF specific issue though. Moving to FF --> Web Apps.
Comment 2 Jason Smith [:jsmith] 2012-04-18 10:41:09 PDT
Not seeing this behavior with a dirty profile on Win 7 64-bit. Flagging qawanted to investigate.
Comment 3 Rob Hawkes [:rob] 2012-04-18 11:09:45 PDT
I see this behaviour on today's Nightly with OSX.
Comment 4 Jason Smith [:jsmith] 2012-04-18 11:42:50 PDT
Confirmed. The whitelisting no longer works. Seen this behavior on OS X 10.6.
Comment 5 Jason Smith [:jsmith] 2012-04-18 11:46:30 PDT
Regression window: Was working on yesterday's nightly, not working on today's nightly. This likely involves the patches that were submitted yesterday on 4/17.
Comment 6 Jason Smith [:jsmith] 2012-04-18 16:03:08 PDT
I think this happens only with a clean FF profile. Does not occur with a dirty profile.
Comment 7 Jason Smith [:jsmith] 2012-04-18 16:03:29 PDT
For comment 6 - Only if the dirty profile already had the whitelisting parameter set.
Comment 8 Ian Bicking (:ianb) 2012-04-19 10:30:07 PDT
Looking at the whitelisting code, it's pretty fragile.  The value must be of exactly the format: https://myapps.mozillalabs.com,https://somethingelse.com,... – each item must be a valid URL, separated with commas if you include multiple domains.  If an item is invalid that item and the rest after it are ignored and no error is reported.  Is there a chance you set the property differently?
Comment 9 JStagner@Mozilla.com 2012-04-19 10:35:40 PDT
In my case I used exactly that url, all lower case - tried 3 machines all with the same failure.
Comment 10 Ian Bicking (:ianb) 2012-04-19 12:22:55 PDT
Created attachment 616694 [details] [diff] [review]
Fix regression

This code, dom/base/Webapps.jsm line 52, is what is stopping the whitelist from being loaded:

    this.appsFile = FileUtils.getFile(DIRECTORY_NAME, ["webapps", "webapps.json"], true);

    if (!this.appsFile.exists())
      return;

So probably if you install an app, then restart your browser, the whitelist will start working.  I've attached a patch.
Comment 11 Ian Bicking (:ianb) 2012-04-19 13:15:50 PDT
Comment on attachment 616694 [details] [diff] [review]
Fix regression

This patch fixes a logic bug, that keeps the mozApps.mgmt whitelist from being loaded in profiles that otherwise have never used mozApps.install()

Risks? None that I see; the remainder of the code block is already protected by a try/catch
Comment 12 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-20 15:44:57 PDT
Comment on attachment 616694 [details] [diff] [review]
Fix regression

[triage comment]
not related to Fennec at all, fine for a a=desktop-only
Comment 13 Myk Melez [:myk] [@mykmelez] 2012-04-20 16:07:10 PDT
lsblakk: I'm ready to check this in, but are you sure this is a=desktop-only?  dom/base/Webapps.jsm is loaded by Fennec; and the tree rules say only browser/ qualifies for that kind of approval (although akeybl recently extended it to webapprt/, which is only built with browser/).
Comment 14 Myk Melez [:myk] [@mykmelez] 2012-04-20 16:43:23 PDT
Comment on attachment 616694 [details] [diff] [review]
Fix regression

Possible risk to Fennec Native 14 (if any):

This fixes a bug, also present in Fennec, which seems low-risk for two reasons:

1. There is no obvious benefit to relying on the current buggy behavior, so it seems unlikely for there to be any code in Fennec that depends on it.

2. The behavior is only triggered when websites access the mozApps API for interacting with Fennec's webapps feature, and webapps are not (yet!) a key feature of Fennec nor exposed much to its users.
Comment 15 Myk Melez [:myk] [@mykmelez] 2012-04-20 16:45:25 PDT
(Note: re-requested approval after conversation with Lukas on IRC and agreement that this isn't desktop-only and thus not subject to a=desktop-only.)
Comment 16 Myk Melez [:myk] [@mykmelez] 2012-04-21 14:17:59 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d48034b460e
Comment 17 Phil Ringnalda (:philor) 2012-04-21 23:28:51 PDT
https://hg.mozilla.org/mozilla-central/rev/0d48034b460e

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