Add link to Mozilla plugin check from Add-ons Manager

VERIFIED FIXED in mozilla6

Status

()

Toolkit
Add-ons Manager
--
enhancement
VERIFIED FIXED
7 years ago
5 years ago

People

(Reporter: kev, Assigned: peregrino)

Tracking

2.0 Branch
mozilla6
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-litmus ?

Firefox Tracking Flags

(firefox6-)

Details

(URL)

Attachments

(6 attachments, 3 obsolete attachments)

(Reporter)

Description

7 years ago
Created attachment 531612 [details]
Mockup of link to plugin check from add-ons manager

Adding plugin check functionality is a goal for the Add-ons Manager which requires some development work (see bug 613305). In the interim between that feature being ready, we should add a link to Mozilla's plugin check page (e.g. https://www.mozilla.com/en-US/plugincheck) from the Plugins section of the Add-ons Manager. Until there is plugincheck functionality in Firefox proper, this will provide users with a simple method to determine if their plugins are up-to-date.

Additional information for this feature can be found on the Wiki at https://wiki.mozilla.org/Features/Firefox/PlugincheckLinkInAOM

UX has provided a mockup (see attached), and we'll need to make l10n aware as well.
One problem I see here is that that text is extremely long and we're going to run into problems as the window is resized unless we crop it in that case

Comment 2

7 years ago
There are two lengthy strings that compete for screen estate in localized build in particular, the plugin check string on top, and the "Recent Updates" string in the tab selector on the left, which in German is "Zuletzt durchgeührte Updates", and leads to a lot of (usually blank) space on the left.

Also, does this depend on merging some of the info of about:plugins into the addons manager? The tab on the left looks like it.

I'm not really sold on this design. Also unclear on when this link should be shown, i.e., does that depend on the tab selection?
(Reporter)

Comment 3

7 years ago
(In reply to comment #1)
> One problem I see here is that that text is extremely long and we're going
> to run into problems as the window is resized unless we crop it in that case

Do we want to consider (and can we) consider redirecting only if the user checks for updates from the plugin page through the settings button? Would adding the check as a list item make it simpler with the wrapping it does?

(In reply to comment #2)
> Also, does this depend on merging some of the info of about:plugins into the
> addons manager? The tab on the left looks like it.

I don't think it does, the intent is to just give users a way to check if their installed plugins are up to date via the plugin check page.

> I'm not really sold on this design. Also unclear on when this link should be
> shown, i.e., does that depend on the tab selection?

It should, I think.
Created attachment 531687 [details]
Mockup of link to plugin check as drop-down notification in add-ons manager

(In reply to comment #3)
> (In reply to comment #1)
> > One problem I see here is that that text is extremely long and we're going
> > to run into problems as the window is resized unless we crop it in that case
> 
> Do we want to consider (and can we) consider redirecting only if the user
> checks for updates from the plugin page through the settings button? Would
> adding the check as a list item make it simpler with the wrapping it does?

As a pure list item with similar height, it could be easily confused as a kind of plugin check-service add-on.  However, if string length is the crux of the issue in this temporary solution, we could display it as we display notifications within list view: as a notification drop-down at the top of the list (see attached).

> (In reply to comment #2)
> > Also, does this depend on merging some of the info of about:plugins into the
> > addons manager? The tab on the left looks like it.
> 
> I don't think it does, the intent is to just give users a way to check if
> their installed plugins are up to date via the plugin check page.
> 
> > I'm not really sold on this design. Also unclear on when this link should be
> > shown, i.e., does that depend on the tab selection?
> 
> It should, I think.

It would depend on tab selection.  So switching tabs mean the link appears and then disappears.  It's weird but temporary behavior, perhaps mitigated by showing the link within list view rather than outside of it, as in the attached.

The only other notification that appears here, as far as I know, says that add-ons are disabled due to safe mode.  So there may be two notifications showing in that case.
Created attachment 531688 [details]
Mockup of link to plugin check as drop-down notification in add-ons manager
Attachment #531687 - Attachment is obsolete: true
I think that second option works pretty well actually
Jennifer, how would that look like with safe mode enabled or the disabled compatibility checks?
(In reply to comment #7)
> Jennifer, how would that look like with safe mode enabled or the disabled
> compatibility checks?

When would compatibility checks be disabled?  Safe mode?
Assignee: nobody → colmeiro
(In reply to comment #8)
> When would compatibility checks be disabled?  Safe mode?

Wait, we have disabled that bar for the plugin pane. So it wouldn't affect us here.

Comment 10

7 years ago
Yeah, the second mockup is much better.

Still confused to see a "plugins" tab showing a host of add-ons plus a pluginchecker notification. For which tab should we show the pluginchecker?
Created attachment 531701 [details]
Screenshot: Currently plugins are not disabled in safe mode

(In reply to comment #7)
> Jennifer, how would that look like with safe mode enabled or the disabled
> compatibility checks?

Right now, it appears plugins aren't disabled in safe mode (see attached).  Is this by design or by accident?  If it's design, no problem, no double notifications!  If it's by accident, we at least don't have a problem until this gets fixed...
(In reply to comment #9)
> (In reply to comment #8)
> > When would compatibility checks be disabled?  Safe mode?
> 
> Wait, we have disabled that bar for the plugin pane. So it wouldn't affect
> us here.

Oddly fortunate, I suppose... 

(In reply to comment #10)
> Yeah, the second mockup is much better.
> 
> Still confused to see a "plugins" tab showing a host of add-ons plus a
> pluginchecker notification. For which tab should we show the pluginchecker?

Is your concern that this link should be in a global management area of UI rather than displayed alongside plugins?  If so, I'd disagree - both because we have no global management UI, and because the check link takes users directly to a list of their own plugins, which I'd argue relates directly to this screen and no others.  Also, once we properly integrate plugin check, it would be available as an action per plugin item.

Comment 13

7 years ago
Oops, I got confused by the mockup showing the plugins tab at a different location where it is right now, and the pane just showing add-ons. We have a plugins section, and yes, showing the notification box there should work out fine.

Technical question, can we allow the text to wrap in that box for small windows and long localizations?
(In reply to comment #13)
> Oops, I got confused by the mockup showing the plugins tab at a different
> location where it is right now, and the pane just showing add-ons. We have a
> plugins section, and yes, showing the notification box there should work out
> fine.
> 
> Technical question, can we allow the text to wrap in that box for small
> windows and long localizations?

I believe so yes.
Created attachment 531732 [details]
Mockup of link to plugin check as drop-down notification in add-ons manager with additional notification that user is in safe mode

(In reply to comment #9)
> (In reply to comment #8)
> > When would compatibility checks be disabled?  Safe mode?
> 
> Wait, we have disabled that bar for the plugin pane. So it wouldn't affect
> us here.

Mossop points out that this is a problem being addressed in bug 342333.  Assuming that this bug gets fixed, we'd still need a way to show (groan) two notifications - one for safe mode and one for the plugin update link.  The incompatible add-on notification is currently hidden when the safe mode notification is present, and this behavior is fine - the former is a fairly rare user-set preference that is constantly activated, while safe mode is usually a one-off run.  Luckily, both of these are user-set, so a normal user who does not change preferences would only ever see the notification for plugin updating and would never run into a double-notification scenario.
Jennifer, can you please update your mockups so they reflect the latest state of the add-ons manager, specifically colors and styles? That would be really appreciated.
Created attachment 531754 [details]
Mockup of link to plugin check as drop-down notification in add-ons manager with additional notification, current UI (Windows 7 5.11.11)

(In reply to comment #16)
> Jennifer, can you please update your mockups so they reflect the latest
> state of the add-ons manager, specifically colors and styles? That would be
> really appreciated.

Sure
(In reply to comment #11)
> Right now, it appears plugins aren't disabled in safe mode (see attached). 
> Is this by design or by accident?  If it's design, no problem, no double
> notifications!  If it's by accident, we at least don't have a problem until
> this gets fixed...

A little bit of both. Right now, plugins don't get disabled in safe-mode - so it's intentional that we don't show the safe-mode warning in the plugins pane (bug 492271). However, bug 342333 will eventually make it so plugins are disabled in safe-mode.
Created attachment 531986 [details] [diff] [review]
initial patch

Well, first approach to solve this. I'm attaching a proposed patch, but have a few questions:

* I Don't know if I chose the right entities for the message, nor if the strings are the correct ones.

* the screenshot attached to comment #11 shows that when in safemode the "addons disabled" message is not properly hidden. Fixing that too.

* Don't know if I chose the right names for the classes for the container of the message, as I think this is a new kind of notification there.

* When coding the cmd_ action, I noticed that in extension.js some preferences are used with constants, but others are just accessed directly. Is this OK?
Attachment #531986 - Flags: review?(dtownsend)

Updated

7 years ago
tracking-firefox5: --- → ?
We don't track features until after they have landed.
tracking-firefox5: ? → ---
tracking-firefox6: ? → ---
Comment on attachment 531986 [details] [diff] [review]
initial patch

Review of attachment 531986 [details] [diff] [review]:
-----------------------------------------------------------------

This is great. Only one trivial comment on the patch so far however it isn't quite finished yet. We should have an automated test for this, it should check that the message is visible on the plugins pane and not on another one and that clicking on it does open the correct webpage. I'd also like you to sit down with Boriss and show her what it looks like right now and find out what styling changes she'd like. You'll probably have to trigger a try build once you have the test written and you can use the generated builds to see what it looks like on other platforms.

::: toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd
@@ +22,5 @@
>  <!ENTITY warning.updatesecurity.label              "Add-on update security checking is disabled. You may be compromised by updates.">
>  <!ENTITY warning.updatesecurity.enable.label       "Enable">
>  <!ENTITY warning.updatesecurity.enable.tooltip     "Enable add-on update security checking">
> +<!ENTITY warning.plugincheck.label                 "Check to see if your Plugins are up to date">
> +<!ENTITY warning.plugincheck.tooltip               "Check to see if your Plugins are up to date">

I would use info.plugincheck.* here just because as you've rightly done elsewhere these aren't warning messages.
Attachment #531986 - Flags: review?(dtownsend) → review-
Created attachment 532130 [details] [diff] [review]
added automated test

changed the strings location to info.plugincheck.* and added automated test
Attachment #531986 - Attachment is obsolete: true
Attachment #532130 - Flags: review?(dtownsend)
Comment on attachment 532130 [details] [diff] [review]
added automated test

Review of attachment 532130 [details] [diff] [review]:
-----------------------------------------------------------------

Couple of tweaks for the test, otherwise the code is all good, going to wait to see the styling parts before granting this review though.

::: toolkit/mozapps/extensions/test/browser/browser_globalinformations.js
@@ +43,5 @@
> +  gLoadCompleteCallback = function() {
> +    browser.removeProgressListener(gProgressListener);
> +    aCallback(aManager);
> +  };
> +}

There is actually a simpler way to do this without the need for the progress listener, look here: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/browser/browser_purchase.js#139

@@ +58,5 @@
> +      var button = aManager.document.querySelector("#list-view hbox.global-info-plugincheck button.button-link");
> +      is_element_visible(button, "Plugin Check message button should be visible");
> +
> +      info("Clicking 'Plugin Check' button");
> +      EventUtils.synthesizeMouse(button, 2, 2, { }, aManager);

Use synthesizeMouseAtCenter here
Attachment #532130 - Flags: review?(dtownsend)
Created attachment 533023 [details] [diff] [review]
patch with corrections
Attachment #532130 - Attachment is obsolete: true
Attachment #533023 - Flags: review?(dtownsend)
Attachment #533023 - Flags: ui-review?(jboriss)
Comment on attachment 533023 [details] [diff] [review]
patch with corrections

This looks great!  A couple changes that have been made here (discussed via email):  

- Background color for OSX changed to #e3e6eb
- Background color for Windows changed to #f3f7fb
- String capitalization corrected ("plugins" in lowercase)
Attachment #533023 - Flags: ui-review?(jboriss) → ui-review+

Comment 26

7 years ago
Is this going to come in Firefox 5?
Comment on attachment 533023 [details] [diff] [review]
patch with corrections

Review of attachment 533023 [details] [diff] [review]:
-----------------------------------------------------------------

Great work!
Attachment #533023 - Flags: review?(dtownsend) → review+
Landed: http://hg.mozilla.org/mozilla-central/rev/e816b1abd0e0

(In reply to comment #26)
> Is this going to come in Firefox 5?

Firefox 5 is already in beta testing, so no it is unlikely to make it there.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [wanted-fx6]
Target Milestone: --- → mozilla6
(Reporter)

Comment 29

7 years ago
Nicely done, Hernán. Thanks for pushing this through, and thanks to all for your input and help.

(In reply to comment #26)
> Is this going to come in Firefox 5?

Nope. It should be in the following release if it lands in the next week (I believe).
tracking-firefox6: --- → ?
Depends on: 658275

Updated

7 years ago
Depends on: 659111

Updated

7 years ago
tracking-firefox6: ? → -
Verified fixed across platforms with builds like Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0a2) Gecko/20110529 Firefox/6.0a2 ID:20110529042002

It would have been great if the link element would have gotten an id assigned to. I will raise a follow-up bug.
Status: RESOLVED → VERIFIED
Comment on attachment 533023 [details] [diff] [review]
patch with corrections

>+            <hbox class="global-info-plugincheck" flex="1" align="center"
>+                  tooltiptext="Update plugins">
>+              <button class="button-link"
>+                      label="&info.plugincheck.label;"
>+                      tooltiptext="&info.plugincheck.tooltip;"

I will also take care of this hard-coded tooltiptext property which seems to be not necessary at all.
Oh, and I think we should have a manual test to at least test that we are able to open the live web page and show its content. That's not something we can test with the automated test.
Flags: in-litmus- → in-litmus?
Bug 661134 is now covering the localization of the plugincheck web page.
Depends on: 661134

Updated

7 years ago
Blocks: 662246

Comment 34

5 years ago
xref Bug 929474 Add plugin check natively into Firefox
You need to log in before you can comment on or make changes to this bug.