Closed Bug 392084 Opened 13 years ago Closed 13 years ago

Add a get plugins link to the add-ons mgr.

Categories

(Toolkit :: Add-ons Manager, enhancement)

enhancement
Not set

Tracking

()

VERIFIED FIXED
mozilla1.9alpha8

People

(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)

References

Details

Attachments

(1 file, 1 obsolete file)

As we have for extensions and themes.
Summary: Add a get plugins link to the add-ons mgr. → Add a get plugins and get locales links to the add-ons mgr.
OS: Windows Vista → All
Hardware: PC → All
Attached patch patch rev1 (obsolete) — Splinter Review
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
Comment on attachment 279061 [details] [diff] [review]
patch rev1

Can I assume a new patch is going to come later to address the "get locales" part of this bug?
Please make sure that we're consisten about the spelling of "Plugins"

Do we spell it like in about:plugins : "Installed plug-ins"

Or like at:
https://addons.mozilla.org/en-US/firefox/browse/type:7 "Plugins"

??
(In reply to comment #2)
> (From update of attachment 279061 [details] [diff] [review])
> Can I assume a new patch is going to come later to address the "get locales"
> part of this bug?
I'll probably file a separate bug for that since AMO doesn't have a languages (e.g. locales) section yet.

(In reply to comment #3)
> Please make sure that we're consisten about the spelling of "Plugins"
> 
> Do we spell it like in about:plugins : "Installed plug-ins"
> 
> Or like at:
> https://addons.mozilla.org/en-US/firefox/browse/type:7 "Plugins"
> 
> ??
We use Plugins in primary UI (e.g. same as the Plugins view in the Add-ons Manager) per the last time this came up and we discussed it to death in the newsgroups.
Filed spinoff Bug 394704 for locales.
Summary: Add a get plugins and get locales links to the add-ons mgr. → Add a get plugins links to the add-ons mgr.
Summary: Add a get plugins links to the add-ons mgr. → Add a get plugins link to the add-ons mgr.
Attached patch patchSplinter Review
I'll file followup bugs for all toolkit apps and let them decide whether they want to add this.
Attachment #279061 - Attachment is obsolete: true
Attachment #279408 - Flags: review?(dtownsend)
Comment on attachment 279408 [details] [diff] [review]
patch

Code is good as it is, but a few changes that you might want to make.

>Index: toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties

> extensions.update.url=https://addons.mozilla.org/update/VersionCheck.php?reqVersion=%REQ_VERSION%&id=%ITEM_ID%&version=%ITEM_VERSION%&maxAppVersion=%ITEM_MAXAPPVERSION%&status=%ITEM_STATUS%&appID=%APP_ID%&appVersion=%APP_VERSION%&appOS=%APP_OS%&appABI=%APP_ABI%
> extensions.getMoreExtensionsURL=https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%/%VERSION%/extensions/
> extensions.getMoreThemesURL=https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%/%VERSION%/themes/
>+extensions.getMorePluginsURL=https://%LOCALE%.add-ons.mozilla.com/%LOCALE%/%APP%/%VERSION%/plugins/

Why are we maintaining these urls in the locale? They don't appear to be used in the UI from what I can see.
On a related note why do we allow the locale to override extensions.update.url? I came across one locale the other day that was doing so with an incorrect url.

>Index: toolkit/mozapps/extensions/content/extensions.js

>   switch (aView) {
>     case "extensions":
>+      prefURL = PREF_EXTENSIONS_GETMOREEXTENSIONSURL;
>       var types = [ [ ["type", nsIUpdateItem.TYPE_EXTENSION, "Integer"] ] ];
>       break;
>     case "themes":
>+      prefURL = PREF_EXTENSIONS_GETMORETHEMESURL;
>       types = [ [ ["type", nsIUpdateItem.TYPE_THEME, "Integer"] ] ];
>       break;
>     case "locales":
>       types = [ [ ["type", nsIUpdateItem.TYPE_LOCALE, "Integer"] ] ];
>       break;
>     case "plugins":
>+      prefURL = PREF_EXTENSIONS_GETMOREPLUGINSURL;
>       types = [ [ ["plugin", "true", null] ] ];
>       break;

I'm wondering whether we should hide the install button in the plugins view since we can't really install a plugin in this way anymore.

>+  if (aView == "themes" || aView == "extensions") {
>+    var el = document.getElementById("installFileButton");
>+    el.setAttribute("tooltiptext", el.getAttribute(isThemes ? "tooltiptextthemes" :
>+                                                              "tooltiptextaddons"));
>+    el = document.getElementById("checkUpdatesAllButton");
>+    el.setAttribute("tooltiptext", el.getAttribute(isThemes ? "tooltiptextthemes" :
>+                                                              "tooltiptextaddons"));
>   }

Do we want a special tooltip on the locales pane? I think it's probably fine with just add-on since we don't discriminate what we install, but in that case I think the file picker title should be changed from extension to add-on.
Attachment #279408 - Flags: review?(dtownsend) → review+
(In reply to comment #7)
>...
> Why are we maintaining these urls in the locale? They don't appear to be used
> in the UI from what I can see.
> On a related note why do we allow the locale to override extensions.update.url?
> I came across one locale the other day that was doing so with an incorrect url.
I plan on dealing with that after M8 by reviewing all the strings.

> I'm wondering whether we should hide the install button in the plugins view
> since we can't really install a plugin in this way anymore.
UX has mentioned they are against hiding buttons (e.g. Restart Button when it is not applicable).

> Do we want a special tooltip on the locales pane? I think it's probably fine
> with just add-on since we don't discriminate what we install, but in that case
> I think the file picker title should be changed from extension to add-on.
Yes as well as fixing the one on the updates button in the Plugins view even though it is disabled.

I'll file a followup bug after I've had a chance to go through the UI for other issues like this.
Checked in to trunk

Checking in mozilla/browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.200; previous revision: 1.199
done
Checking in mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties,v  <--  extensions.properties
new revision: 1.39; previous revision: 1.38
done
Checking in mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd,v  <--  extensions.dtd
new revision: 1.25; previous revision: 1.24
done
Checking in mozilla/toolkit/mozapps/extensions/content/extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v  <--  extensions.js
new revision: 1.144; previous revision: 1.143
done
Checking in mozilla/toolkit/mozapps/extensions/content/extensions.xul;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.xul,v  <--  extensions.xul
new revision: 1.62; previous revision: 1.61
done
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M8
Filed spinoff bugs
Bug 394813 - for the Find Updates button
Bug 394814 - for extensions.update.url
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X; es-ES; rv:1.9a8pre) Gecko/2007090404 Minefield/3.0a8pre.  Link navigates to: https://addons.mozilla.org/en-US/firefox/browse/type:7
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.