Closed Bug 1403751 Opened 7 years ago Closed 7 years ago

Tell users how to enable a disabled extension in about:preferences

Categories

(WebExtensions :: Frontend, defect, P3)

Product:
WebExtensions
Component:
Frontend
Type:
defect

Tracking

(firefox59 verified)

Status:
VERIFIED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- verified

People

(Reporter: mstriemer, Assigned: mstriemer)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Bug 1403751 - Tell users how to enable extensions in about:preferences
59 bytes, text/x-review-board-request
jaws
: review+
Details
enable-message.mov.gif
186.38 KB, image/gif
Details
disable webext.gif
112.80 KB, image/gif
Details 
Users are notified of extensions that are controlling some settings in Firefox. Sometimes this notice includes a disable button.

When the user disables the last extension controlling a setting this message should be replaced with another message that tells the user how to enable the extension again.

Mocks: https://mozilla.invisionapp.com/share/6HCITJKP8#/screens/246507430
There could be multiple extensions enabling that preference. If I've got 3 disabled extensions that could be controlling that, how does it show? If I got 2 disabled and 1 enabled, does it just show the enabled extension only?
Blocks: 1342584
Priority: -- → P3
(In reply to Andy McKay [:andym] from comment #1)
> There could be multiple extensions enabling that preference. If I've got 3
> disabled extensions that could be controlling that, how does it show?

Good question. I suppose we could just show the one with the highest precedence, but I'm not sure that makes sense. It seems like we should either show them all, or none of them.

> If I
> got 2 disabled and 1 enabled, does it just show the enabled extension only?

In this case I think the answer is "yes", it will just show you the extension that is currently controlling the setting.
You see the extension that is currently controlling the preference.

1. Install "New Tab 1", it is shown in about:preferences.
2. Install "New Tab 2", it is shown instead of "New Tab 2".
3. Click Disable Extension, "New Tab 1" is shown again.
4. Click Disable Extension, this message is now shown.

It might make sense to show all of them but this is how it was mocked up for now.
Attached image enable-message.mov.gif — 
Short gif of the interaction for a New Tab extension.
Comment on attachment 8925717 [details]
Bug 1403751 - Tell users how to enable extensions in about:preferences

https://reviewboard.mozilla.org/r/196858/#review203392

r- due to question about usefulness of "X" button, as well as lacking the name of the extension.

::: browser/components/preferences/in-content/preferences.js:421
(Diff revision 1)
> +  let message = document
> +    .getElementById("bundlePreferences")
> +    .getFormattedString("extensionControlled.enable", [addonIcon, toolbarIcon]);

We should include the name of the extension here. It looks like the only way that extensionControlledIds has values is if the extension was previously enabled, then disabled, so we should be able to cache that name while it is enabled.

::: browser/components/preferences/in-content/preferences.js:430
(Diff revision 1)
> +  dismissButton.addEventListener("click", function dismissHandler() {
> +    hideControllingExtension(settingName);
> +    dismissButton.removeEventListener("click", dismissHandler);

As a user I would expect that dismissing this will permanently hide the notice. But with this patch reloading the page though will bring it back.

Since it's not persistent I would prefer we remove the X to dismiss.

::: browser/components/preferences/in-content/preferences.js:435
(Diff revision 1)
> +  let disableButton = extensionControlledContent.querySelector("description + button");
> +  if (disableButton) {
> +    disableButton.hidden = true;
> +  }

This should be done by CSS that is keyed off of the .extension-controlled-disabled class you added above on line 417.

::: browser/locales/en-US/chrome/browser/preferences/preferences.properties:294
(Diff revision 1)
> +# LOCALIZATION NOTE (extensionControlled.enable):
> +# %1$S is replaced with the icon for the add-ons menu.
> +# %2$S is replaced with the icon for the toolbar menu.
> +# This string is shown to notify the user how to enable an extension that they disabled.
> +# Note, this string will be used as raw markup. Avoid characters like <, >, &
> +extensionControlled.enable = To enable the extension go to %1$S Add-ons in the %2$S menu.

Please get sign-off from Michelle Heubusch as to the wording here.

Right now this wording doesn't explain why the message is where it appears. But if this message only shows up after the user has clicked to "disable" (but not reloaded the page) then it would be fine.

If the message shows up on new visits to the page, then it will be confusing since it doesn't say which extension nor why this extension is related to the home page.

::: browser/themes/shared/incontentprefs/preferences.inc.css:186
(Diff revision 1)
> +  fill-opacity: 1;
> +  stroke-opacity: 1;

fill-opacity:1 and stroke-opacity:1 are the default values and can be omitted.

::: browser/themes/shared/incontentprefs/preferences.inc.css:191
(Diff revision 1)
> +  fill-opacity: 1;
> +  stroke-opacity: 1;
> +}
> +
> +.extension-controlled-icon-close {
> +  fill-opacity: 0;

You can use `fill: transparent;` here instead of changing fill-opacity, which will then allow you to remove the fill-opacity from the -moz-context-properties definition.
Attachment #8925717 - Flags: review?(jaws) → review-
Comment on attachment 8925717 [details]
Bug 1403751 - Tell users how to enable extensions in about:preferences

https://reviewboard.mozilla.org/r/196858/#review203392

> We should include the name of the extension here. It looks like the only way that extensionControlledIds has values is if the extension was previously enabled, then disabled, so we should be able to cache that name while it is enabled.

We could do that but I think the idea was that we didn't need it since the user just saw the name of the extension when they clicked "Disable Extension".

This could be confusing if the user opens about:preferences and about:addons and disables the extension in about:addons, then switches to about:preferences. This doesn't seem like an overly likely use case to account for though.

> As a user I would expect that dismissing this will permanently hide the notice. But with this patch reloading the page though will bring it back.
> 
> Since it's not persistent I would prefer we remove the X to dismiss.

I had asked Markus about the X before, since I didn't think we needed it. I figured we didn't need it because reloading the page removes the message. Which is what I see when I use this patch, so I'm not totally sure what you mean by "reloading the page though will bring it back."

Here's what Markus said:

> Yes, we need it. The X is there to indicate that this is a temporary message. 
> Without it users might think that message will stay, and might expect it to be around when they come back the next time... 
> (but then it will be gone.)

> Please get sign-off from Michelle Heubusch as to the wording here.
> 
> Right now this wording doesn't explain why the message is where it appears. But if this message only shows up after the user has clicked to "disable" (but not reloaded the page) then it would be fine.
> 
> If the message shows up on new visits to the page, then it will be confusing since it doesn't say which extension nor why this extension is related to the home page.

I don't see the message when I reload the page so I'm not sure what the concern is here. There are two ways that I expect this message to appear, but if you reload about:preferences it should definitely be gone. If you still see it that would be a bug.

The common case would be that you clicked "Disable Extension" so you've got this extra context.

The uncommon case would be you happened to have about:preferences open and the extension gets disabled by something else. Maybe the New Tab doorhanger or through about:addons or something. Switching back to about:preferences at this point would show the message.

The second case isn't intentional but didn't seem like a huge bother. We could hide these messages on tab focus if that is better.

> You can use `fill: transparent;` here instead of changing fill-opacity, which will then allow you to remove the fill-opacity from the -moz-context-properties definition.

It looks like I need this from my testing. This icon is basically a filled square with the X being the stroke, as far as I can tell. So setting `fill: transparent` makes it disappear. Perhaps I need to change something else too?
We've got some more UX questions on this one, Markus.
Flags: needinfo?(mjaritz)
I am not sure about what the open questions are, but I will just comment on everything UX related.


> > We should include the name of the extension here. 
> 
> We could do that but I think the idea was that we didn't need it since the
> user just saw the name of the extension when they clicked "Disable
> Extension".

That's right, we do not need it, as this message should only show if one just removed that extension by clicking the remove button in preferences. (If they removed the extension in any other way, this message should just be gone. - As no extension is overriding the pref anylonger.)


> This could be confusing if the user opens about:preferences and about:addons
> and disables the extension in about:addons, then switches to
> about:preferences. This doesn't seem like an overly likely use case to
> account for though.

And they then already know what that message would have told them. So no need to show it.


> > Please get sign-off from Michelle Heubusch as to the wording here.

Michelle reviewed the copy in the Mock-ups, and where needed I changed it.


> > Right now this wording doesn't explain why the message is where it appears. But if this message only shows up after the user has clicked to "disable" (but not reloaded the page) then it would be fine.

That is the idea.


> > If the message shows up on new visits to the page, then it will be confusing since it doesn't say which extension nor why this extension is related to the home page.

That shouldn't happen.


> The uncommon case would be you happened to have about:preferences open and
> the extension gets disabled by something else. Maybe the New Tab doorhanger
> or through about:addons or something. Switching back to about:preferences at
> this point would show the message.
> 
> The second case isn't intentional but didn't seem like a huge bother. We
> could hide these messages on tab focus if that is better.

Yes, please hide the message if it the extension got disabled in any other way than through the button in preferences.


> It looks like I need this from my testing. This icon is basically a filled
> square with the X being the stroke, as far as I can tell. So setting `fill:
> transparent` makes it disappear. Perhaps I need to change something else too?

This icon looks different from how I know it: http://design.firefox.com/icons/viewer/#close%2016
That difference might be to allow for the hover effect: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/close-icon.inc.css#15 (Can we please use the same hover effect...)
Flags: needinfo?(mjaritz)
Comment on attachment 8925717 [details]
Bug 1403751 - Tell users how to enable extensions in about:preferences

https://reviewboard.mozilla.org/r/196858/#review207576
Attachment #8925717 - Flags: review?(jaws) → review+
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/833a6bc4eb72
Tell users how to enable extensions in about:preferences r=jaws
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/833a6bc4eb72
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Attached image disable webext.gif — 
I can reproduce this issue on Firefox 58.0a1 (20170927100120) under Wind 10 64-bit. 

This issue is verified as fixed on Firefox 59.0a1 (20171206100053) under Wind 10 64-bit and Mac OS X 10.13.
Status: RESOLVED → VERIFIED
status-firefox59: fixed → verified
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: