Closed Bug 348842 Opened 15 years ago Closed 15 years ago

Make get-more-dictionaries use the canonical localized-service URL format

Categories

(Firefox :: General, defect)

2.0 Branch
defect
Not set
major

Tracking

()

RESOLVED FIXED
Firefox 2 beta2

People

(Reporter: shaver, Assigned: mwu)

References

Details

(Keywords: fixed1.8.1)

Attachments

(1 file, 1 obsolete file)

See also 343076.  I don't know if you want to use www or add-ons as the service; depends on where you want primary control for the disposition to rest.
Boldly marking as blocking beta2, per yesterday's conversations.
Flags: blocking-firefox2+
Target Milestone: --- → Firefox 2 beta2
What's the canonical service name for add-ons? add-ons or amo?
Status: NEW → ASSIGNED
Assignee: nobody → beltzner
Status: ASSIGNED → NEW
Dietrich, is the following right in order to produce a URL like:

http://en-US.add-ons.mozilla.com/en-US/firefox/2.0/dictionaries/

First, in browser/app/profile/firefox.js at line 80:

-pref("browser.dictionaries.download.url", "https://addons.mozilla.org/%LOCALE%/firefox/%VERSION%/dictionaries/");
+pref("browser.dictionaries.download.url", "https://%LOCALE%.%SERVICE%.mozilla.com/%LOCALE%/firefox/%VERSION%/dictionaries/");

and then, if I understand things right, it would be in /browser/base/content/browser.js at line 5289:

-      var uri = gPrefService.getCharPref("browser.dictionaries.download.url");
+      var uri = formatURL(gPrefService.getCharPref("browser.dictionaries.download.url"), {SERVICE: add-ons});

That about right?
Assignee: beltzner → michael.wu
(In reply to comment #3)
> Dietrich, is the following right in order to produce a URL like:
> 
> http://en-US.add-ons.mozilla.com/en-US/firefox/2.0/dictionaries/
> 
> First, in browser/app/profile/firefox.js at line 80:
> 
> -pref("browser.dictionaries.download.url",
> "https://addons.mozilla.org/%LOCALE%/firefox/%VERSION%/dictionaries/");
> +pref("browser.dictionaries.download.url",
> "https://%LOCALE%.%SERVICE%.mozilla.com/%LOCALE%/firefox/%VERSION%/dictionaries/");
> 
> and then, if I understand things right, it would be in
> /browser/base/content/browser.js at line 5289:
> 
> -      var uri = gPrefService.getCharPref("browser.dictionaries.download.url");
> +      var uri =
> formatURL(gPrefService.getCharPref("browser.dictionaries.download.url"),
> {SERVICE: add-ons});
> 
> That about right?
> 

That's correct-ish. The function will get the pref for you if you set the last parameter aIsPref=true:

formatURL("browser.dictionaries.download.url", {SERVICE: "add-ons"}, true);
Attached patch Use formatURL (obsolete) — Splinter Review
Attachment #234103 - Flags: review?(vladimir)
Isn't the site/service "addons", not "add-ons"?  Or is the service called "add-ons"?
Comment on attachment 234103 [details] [diff] [review]
Use formatURL

r=me, based on irc conversation about "add-ons"
Attachment #234103 - Flags: review?(vladimir) → review+
As per discussion on IRC, we shouldn't be using %SERVICE% as a freeform variable. Instead, just hardcode the service name.
Attachment #234103 - Attachment is obsolete: true
Attachment #234169 - Flags: review?(beltzner)
Attachment #234169 - Flags: review?(beltzner)
Attachment #234169 - Flags: review+
Attachment #234169 - Flags: approval1.8.1?
Comment on attachment 234169 [details] [diff] [review]
Hardcode "add-ons" instead

vlad: now that bug 347944 has moved this code to /toolkit, is this patch still correct?
Attachment #234169 - Flags: review+ → review?(vladimir)
Comment on attachment 234169 [details] [diff] [review]
Hardcode "add-ons" instead

mwu tells me that moving the code won't affect this patch, so carrying over r=vlad, and giving a=beltzner on behalf of drivers.

Please only check this in *after* bug 347944 has landed, otherwise you'll break things in naughty ways.
Attachment #234169 - Flags: review?(vladimir)
Attachment #234169 - Flags: review+
Attachment #234169 - Flags: approval1.8.1+
Whiteboard: [checkin needed (1.8 branch)]
Whiteboard: [checkin needed (1.8 branch)] → [checkin needed] [checkin needed (1.8 branch)]
Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.145; previous revision: 1.144
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.684; previous revision: 1.683
done

Waiting for approval/checkin of 347944 on branch before checking this in on branch.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed] [checkin needed (1.8 branch)] → [checkin needed (1.8 branch)]
Keywords: fixed1.8.1
Whiteboard: [checkin needed (1.8 branch)]
You need to log in before you can comment on or make changes to this bug.