537186 - Update some URLs and remove unneeded prefs in mobile.js and default bookmarks

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Reed Loden [:reed]]

Attachment: patch - v1

Comment 1

[Reed Loden [:reed]]

Comment on attachment 419505 [details] [diff] [review]
patch - v1

Walk-through of the patch...

>-pref("extensions.getAddons.browseAddons", "https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%");
>-pref("extensions.getAddons.recommended.browseURL", "https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%/recommended");
>-pref("extensions.getAddons.search.browseURL", "https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/mobile/search?q=%TERMS%");

Don't seem to be used at all from what I can tell.

>-pref("extensions.blocklist.detailsURL", "http://%LOCALE%.www.mozilla.com/%LOCALE%/blocklist/");
>-
>-/* dictionary download preference */
>-pref("browser.dictionaries.download.url", "https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/firefox/%VERSION%/dictionaries/");
>+pref("extensions.blocklist.detailsURL", "https://www.mozilla.com/%LOCALE%/blocklist/");

Remove unused dictionary pref and update blocklist detailsURL to current URL used by Firefox.

>-pref("browser.search.defaultenginename",      "chrome://browser/locale/region.properties");
>+pref("browser.search.defaultenginename", "chrome://browser/locale/region.properties");
>-pref("privacy.item.history",     true);
>+pref("privacy.item.history", true);

Whitespace only changes.

>-// URL to the Learn More link XXX this is the firefox one.  Bug XXX fixes this.
>-pref("browser.geolocation.warning.infoURL", "http://%LOCALE%.www.mozilla.com/%LOCALE%/firefox/geolocation/");
>+// URL to the Learn More link XXX this is the firefox one.  Bug 495578 fixes this.
>+pref("browser.geolocation.warning.infoURL", "http://www.mozilla.com/%LOCALE%/firefox/geolocation/");

Add bug # to comment and remove locale-based subdomain.

>-pref("app.releaseNotesURL", "http://www.mozilla.org/projects/fennec/%VERSION%/releasenotes/");
>+pref("app.releaseNotesURL", "http://www.mozilla.org/projects/%APP%/%VERSION%/releasenotes/");

My understanding is that %APP% is "fennec" here, but if that's not correct, this change can be left out.

>-pref("app.creditsURL", "http://www.mozilla.com/%LOCALE%/mobile/credits");
>+pref("app.creditsURL", "http://www.mozilla.com/%LOCALE%/mobile/credits/");

Add ending slash to match other URLs.

>-       {"index":3,"title":"@bookmarks_addons@",   "type":"text/x-moz-place", "uri":"https://addons.mozilla.org/@AB_CD@/mobile",
>+       {"index":3,"title":"@bookmarks_addons@",   "type":"text/x-moz-place", "uri":"https://addons.mozilla.org/@AB_CD@/mobile/",

Add ending slash to match other URLs.

>-       {"index":4,"title":"@bookmarks_support@",  "type":"text/x-moz-place", "uri":"https://mobile.support.mozilla.com/@AB_CD@/kb/",
>+       {"index":4,"title":"@bookmarks_support@",  "type":"text/x-moz-place", "uri":"http://mobile.support.mozilla.com/@AB_CD@/kb/",

Remove unneeded use of SSL here. Neither Firefox nor the app.support.url uses SSL for mobile sumo, and I see no reason why it should be used here.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #1)
> (From update of attachment 419505 [details] [diff] [review])
> Walk-through of the patch...
> 
> >-pref("extensions.getAddons.browseAddons", "https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%");
> >-pref("extensions.getAddons.recommended.browseURL", "https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%/recommended");
> >-pref("extensions.getAddons.search.browseURL", "https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/mobile/search?q=%TERMS%");
> 
> Don't seem to be used at all from what I can tell.

These are used by the nsIAddonRepository, which we use for the add-on manager, so I'd rather keep them. See http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/src/nsAddonRepository.js

Comment 3

[Reed Loden [:reed]]

(In reply to comment #2)
> These are used by the nsIAddonRepository, which we use for the add-on manager,
> so I'd rather keep them. See
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/src/nsAddonRepository.js

Yeah, but Fennec doesn't implement the necessary UI for those to actually be useful.

Comment 4

[Reed Loden [:reed]]

Attachment: patch - v2

Comment 5

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #3)
> Yeah, but Fennec doesn't implement the necessary UI for those to actually be
> useful.

nsAddonRepository's getRecommendedURL() and homepageURL both fail gracefully (rely on formatURLPref returning about:blank if the pref doesn't exist), but getSearchURL doesn't (just does a bare getCharPref). If we fixed that, I wouldn't be opposed to removing these, but in an ideal world these would live in all.js anyways (since the stuff that depends on them is in toolkit). Though that's probably not ideal for us because we want "mobile" and not "%APP%" (which is "fennec"), so I guess we'd have to override them anyways if we wanted them to work... bah. Maybe we should just leave them in (fixed).

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 420275 [details] [diff] [review]
patch - v2

>diff --git a/locales/generic/profile/bookmarks.json.in b/locales/generic/profile/bookmarks.json.in

>-       {"index":4,"title":"@bookmarks_support@",  "type":"text/x-moz-place", "uri":"https://mobile.support.mozilla.com/@AB_CD@/kb/",
>+       {"index":4,"title":"@bookmarks_support@",  "type":"text/x-moz-place", "uri":"http://mobile.support.mozilla.com/@AB_CD@/kb/",

We may want just this change in 1.0 if we do an RC3... these links will be around basically forever after we ship, since they're copied to profiles on first run.

Comment 7

[Reed Loden [:reed]]

http://hg.mozilla.org/mobile-browser/rev/50964c1adbd6

Comment 8

[Stuart Parmenter]

Has someone verified each and every one of these pref changes works, both in the client and on the server?  Changing our blocklist url at this stage in the game scares me a bit.

Comment 9

[Reed Loden [:reed]]

(In reply to comment #8)
> Has someone verified each and every one of these pref changes works, both in
> the client and on the server?

We've been deprecating URLs that use locale-based subdomains throughout all Mozilla projects (see bug 398938). None of those changes is really anything new here.

> Changing our blocklist url at this stage in the game scares me a bit.

I didn't change the actual blocklist URL in this bug. That's bug 537180.

Comment 10

[Aakash Desai [:aakashd]]

Most of the links changed in preferences thanks to the patch referenced in comment #7 are verified on build: 

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2pre) Gecko/20100111 Firefox/3.6pre Fennec/1.1a1pre


The only one left to verify is http://www.mozilla.org/projects/%APP%/%VERSION%/releasenotes/ as I'm getting a Page 404 not found currently. Caitlin is verifying this.