App update preference ui favors disabling app update

RESOLVED FIXED in Firefox 10

Status

()

Firefox
Preferences
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: rstrong, Assigned: Steffen Wilberg)

Tracking

(4 keywords)

Trunk
Firefox 10
polish, user-doc-needed, ux-jargon, ux-tone
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 -)

Details

(Whiteboard: [sg:want P3])

Attachments

(3 attachments, 9 obsolete attachments)

The preference ui displays

Automatically check for updates to:
[X] Firefox
[X] Add-ons
[X] Search Engines

When updates to Firefox are found:
O Ask me what I want to do
0 Automatically download and install update
  [X] Warn me if this update will disable any of my add-ons


If someone doesn't want updates to be automatically downloaded they might just read the first couple of items on the page and not notice there are other options since they are several lines further down in the ui and instead choose to disable update checks completely.

I don't necessarily like the ui Windows has for these settings but I do think the order of choices is better than what we have.

Firefox has:
1. enable (with individual settings further down the page) or disable entirely
2. N/A for app update
3. N/A for app update
4. individual settings mentioned previously

Windows has:
1. Install updates automatically (recommended)
2. Download update but let me choose whether to install them
3. Check for updates but let me choose whether to download and install them
4. Never check for updates (not recommended)
Rather late in the game but people disabling app update checks is a "bad thing" so requesting blocking to see if we can't get a better ui for Firefox 4.0
blocking2.0: --- → ?
Keywords: uiwanted
This is another one that hurts, but I don't think we can block on this. I don't think it's affecting a significant number of our users.

Good prefs polish to get if/when we re-write the entire dialog, though. Something akin to comment 0 is what we want, which gives clearer options.
blocking2.0: ? → -
Keywords: polish, ux-jargon, ux-tone
If we want to just quickly invert the two sections, so "When update to Firefox are found:" appears first, that's fine.  Otherwise we'll clean this up later when we refactor all of the options.
(In reply to comment #3)
> If we want to just quickly invert the two sections, so "When update to
> Firefox are found:" appears first, that's fine.  Otherwise we'll clean this
> up later when we refactor all of the options.

Yeah, this would be a quick fix, just switch the order of the two sections.
(Assignee)

Comment 5

6 years ago
Taking.

(In reply to comment #0)
> Windows has:
> 1. Install updates automatically (recommended)
> 2. Download update but let me choose whether to install them
> 3. Check for updates but let me choose whether to download and install them
> 4. Never check for updates (not recommended)

Should we support case #2 (download but not install) at all?
#1 would be app.update.enabled and app.update.auto set to true (default values).
#3 would be app.update.enabled == true and app.update.auto == false.
#4 would be app.update.enabled == false and app.update.auto == false.

What about the rest of the current options:
  Automatically check for updates to:
  [X] Add-ons
  [X] Search Engines
Check for those updates in case #1 and #3, and don't check in #4?
Assignee: nobody → steffen.wilberg
(Assignee)

Updated

6 years ago
Blocks: 600500
In comment #0 I wasn't stating that we should have the exact same options as Windows. It was only an example for UX.
(Assignee)

Comment 7

6 years ago
Created attachment 541216 [details]
screenshot

Robert: too late, you already had me hooked :)
Attachment #541216 - Flags: ui-review?(faaborg)
(Assignee)

Comment 8

6 years ago
Created attachment 541219 [details] [diff] [review]
patch

Comment 9

6 years ago
In 'Check for updates, but let me choose whether to download and install them', should there not be a 'Warn me if this will disable any of my add-ons' checkbox?

Also it would be handy if you threw in a 'Check for updates, and say Firefox is up to date even if it has failed to reach the update server' checkbox. Bug 648967
(In reply to comment #9)
> In 'Check for updates, but let me choose whether to download and install
> them', should there not be a 'Warn me if this will disable any of my
> add-ons' checkbox?
There shouldn't though it would be fine to inform the user that when manually choosing whether to download you will always be informed if there are incompatible add-ons.

> Also it would be handy if you threw in a 'Check for updates, and say Firefox
> is up to date even if it has failed to reach the update server' checkbox.
> Bug 648967
That should not be a preference and is already covered by Bug 648967. Please don't try to get your bugs fixed in other bugs
Comment on attachment 541216 [details]
screenshot

yeah this is clearer than what we currently have, sorry about the delay.

small nit, mentioning if the update is downloaded seems kind of peripheral to the overall choice, and increases the complexity (one would assume that an update must be downloaded before it is installed).

Also it seems like we should mention why we are making our recommendations.  

How about:

(*) Automatically install updates (recommended: improved security)
     [x] warm me if this will disable nay of my add-ons
( ) Check for updates, but let me choose whether to install them
( ) Never check for updates (not recommend: security risk)
Attachment #541216 - Flags: ui-review?(faaborg) → ui-review+
(Assignee)

Comment 12

6 years ago
Created attachment 549507 [details] [diff] [review]
patch v1

With strings from comment 11, and more comments.
Attachment #541219 - Attachment is obsolete: true
Attachment #549507 - Flags: review?(gavin.sharp)
(Assignee)

Comment 13

6 years ago
Created attachment 551108 [details] [diff] [review]
patch v1.1

Unbitrotted, and fixed double accesskey.
Attachment #549507 - Attachment is obsolete: true
Attachment #551108 - Flags: review?(gavin.sharp)
Attachment #549507 - Flags: review?(gavin.sharp)
(Assignee)

Comment 14

6 years ago
updateManual.label has a typo ("not recommend").
(Assignee)

Comment 15

6 years ago
Created attachment 555833 [details] [diff] [review]
patch v1.2

Fixed that typo.
Gavin, should I pick another reviewer?
Attachment #551108 - Attachment is obsolete: true
Attachment #555833 - Flags: review?(gavin.sharp)
Attachment #551108 - Flags: review?(gavin.sharp)
(Assignee)

Comment 16

6 years ago
Created attachment 555843 [details] [diff] [review]
patch v1.3

Faaborg suggested a further tweak in bug 600500 comment 14.
So now we have:
(*) Automatically install updates (recommended: improved security)
    [x] Warn me if this will disable any of my add-ons
( ) Check for updates, but let me choose whether to download and install them
( ) Never check for updates (not recommended: security risk)
Attachment #555833 - Attachment is obsolete: true
Attachment #555843 - Flags: review?(gavin.sharp)
Attachment #555833 - Flags: review?(gavin.sharp)
(Assignee)

Updated

6 years ago
Keywords: uiwanted
(Assignee)

Comment 17

6 years ago
https://hg.mozilla.org/projects/ux/rev/74008670f6ff
(Assignee)

Comment 18

6 years ago
Comment on attachment 555843 [details] [diff] [review]
patch v1.3

Robert, could you review this, please? Your queue is much shorter than Gavin's, and you reported the bug...
Attachment #555843 - Flags: review?(gavin.sharp) → review?(robert.bugzilla)
(Assignee)

Comment 19

6 years ago
Oops, patch v1.2 and 1.3 are identical.
Will update updateCheck.label per bug 600500 comment 14 in the next iteration.
Comment on attachment 555843 [details] [diff] [review]
patch v1.3

Took a quick look and see that you have changed the updateHistory.accesskey entity value without changing the name. Locales like en-GB at the very least need to see this change. Please keep updateHistory.label in sync with the entity name change when you fix the above. Please verify this isn't the case else where as well.

The following text seems rather awkward:
Automatically check for &brandShortName; updates:
   Automatically install updates (recommended: improved security)

I think it would read better as
&brandShortName; updates:
   Automatically install updates (recommended: improved security)

Please run that suggested text by faaborg and if he doesn't agree that's fine.

minusing mainly due to the entity name not being changed and to check with faaborg regarding the text.
Attachment #555843 - Flags: review?(robert.bugzilla) → review-
(In reply to Robert Strong [:rstrong] (do not email) from comment #20)
> I think it would read better as
> &brandShortName; updates:
>    Automatically install updates (recommended: improved security)
> 
> Please run that suggested text by faaborg and if he doesn't agree that's
> fine.

I agree that this reads better too. Just in case you need an opinion from the UX team and Faaborg is busy. :)
(Assignee)

Comment 22

6 years ago
Created attachment 562894 [details] [diff] [review]
patch v1.4

1. Dropped the unnecessary change of updateHistory.accesskey.
2. Bumped enableAddonsUpdate2.label and .accesskey to 3 because I have to change the accesskey. Other entities are either new or unchanged.
3. Used "&brandShortName; updates:" as suggested by comment 20.
4. Renamed the second heading from "Automatically check for updates to" (Add-ons, Search Engines) to "Automatically update:" to make it fit better, and more precise (it not only checks, but also updates silently).
5. Wrapped the two sections into proper groupboxes like almost everywhere else in the pref window, and removed the separators.
Attachment #541216 - Attachment is obsolete: true
Attachment #555843 - Attachment is obsolete: true
(Assignee)

Comment 23

6 years ago
Created attachment 562898 [details]
Linux screenshot of patch v1.4

Sadly there's no border around the groupboxes on Linux, but the captions are bold, and the contents are indented.
Attachment #562898 - Flags: ui-review?(faaborg)
(Assignee)

Comment 24

6 years ago
https://hg.mozilla.org/projects/ux/rev/dce8c0b89e94
(Assignee)

Updated

6 years ago
Attachment #562898 - Attachment description: screenshot of patch v1.4 → Linux screenshot of patch v1.4
(Assignee)

Comment 25

6 years ago
Created attachment 563946 [details]
Windows screenshot of patch v1.5
Attachment #563946 - Flags: ui-review?(faaborg)
(Assignee)

Comment 26

6 years ago
Created attachment 563947 [details] [diff] [review]
patch v1.5

Had to remove the align="start" attribute of the tabpanel element to make the width of the groupboxes extend to the width of the tabpanel. The other tabpanels don't have this attribute either.
Attachment #562894 - Attachment is obsolete: true
Comment on attachment 563946 [details]
Windows screenshot of patch v1.5

This is the patch and not the Windows screenshot
(Assignee)

Comment 28

6 years ago
Created attachment 563948 [details]
Windows screenshot of patch v1.5

Oops.
Attachment #563946 - Attachment is obsolete: true
Attachment #563948 - Flags: ui-review?(faaborg)
Attachment #563946 - Flags: ui-review?(faaborg)
Comment on attachment 563948 [details]
Windows screenshot of patch v1.5

Looks good!
Comment on attachment 562898 [details]
Linux screenshot of patch v1.4

looks good
Attachment #562898 - Flags: ui-review?(faaborg) → ui-review+
Comment on attachment 563948 [details]
Windows screenshot of patch v1.5

looks good
Attachment #563948 - Flags: ui-review?(faaborg) → ui-review+

Comment 32

6 years ago
The Windows screenshot looks very good.  But, shouldn't the line that says "not recommend: security risk" actually say "not recommended: security risk"?  It would flow better with the line above that says "recommended: improved security".
(Assignee)

Comment 33

6 years ago
(In reply to Jeff Grossman from comment #32)
That's a typo, thanks for catching that!
(Assignee)

Comment 34

6 years ago
Created attachment 564662 [details] [diff] [review]
patch v1.6

Typo fixed.
Attachment #563947 - Attachment is obsolete: true
Attachment #564662 - Flags: review?(robert.bugzilla)
Steffen, patch looks good but before I r+ this I am going to run through the different configurations to verify everything works as expected. I will hopefully have this reviewed by tonight but it may be a day or so. Thanks
(Assignee)

Comment 36

6 years ago
Sure, take all the time you need, and thanks for the update.
Note that I (re-)landed this on the UX branch, so there are plenty of "-ux" nightlies to test.
(Assignee)

Comment 37

6 years ago
(In reply to myself from comment #36)
Things not to tell your reviewer:
#1: "take all the time you need" ;-)
I promise, I'm close to being done though I was close last week as well. ;)
Comment on attachment 564662 [details] [diff] [review]
patch v1.6

Looks good and everything worked as expected with a disable updater build. It might be a good thing to have finer control over what controls are disabled when locking prefs now that there is an additional control but I'm not too worried about that and think we should wait to see if providing that would be of any value.

># HG changeset patch
># Parent 70e4de45a0d0f7b54e4dbc22c177e56a9c717a42
># User Steffen Wilberg <steffen.wilberg@web.de>
>Bug 600505: Reorganize app update preference ui to not favor disabling app update. ui-review=faaborg
>
>diff --git a/browser/components/preferences/advanced.js b/browser/components/preferences/advanced.js
>--- a/browser/components/preferences/advanced.js
>+++ b/browser/components/preferences/advanced.js
>...
>   /**
>-   * Enables/disables UI for "when updates are found" based on the values,
>-   * and "locked" states of associated preferences.
>+   * Sets the pref values based on the selected item of the radiogroup,
>+   * and sets the disabled state of the warnIncompatible checkbox accordingly.
>    */
>-  updateAutoItems: function () 
>+  updateWritePrefs: function ()
>   {
>     var enabledPref = document.getElementById("app.update.enabled");
>     var autoPref = document.getElementById("app.update.auto");
>-    
>-    var updateModeLabel = document.getElementById("updateModeLabel");
>-    var updateMode = document.getElementById("updateMode");
>-    
>-    var disable = enabledPref.locked || !enabledPref.value ||
>-                  autoPref.locked;
>-    updateModeLabel.disabled = updateMode.disabled = disable;
>-  },
>+    var radiogroup = document.getElementById("updateRadioGroup");
>+    switch (radiogroup.value) {
>+      case "auto":      // 1. Automatically install updates
>+        enabledPref.value = true;
>+        autoPref.value = true;
>+        break;
>+      case "checkOnly": // 2. Check, but but let me choose
s/but but/but/

r=me with that
Attachment #564662 - Flags: review?(robert.bugzilla) → review+
(Assignee)

Comment 40

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/2d9076dbcd15
Keywords: user-doc-needed
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/2d9076dbcd15
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10

Updated

6 years ago
Whiteboard: [inbound] → [inbound][sg:want P3]
(Assignee)

Updated

6 years ago
Whiteboard: [inbound][sg:want P3] → [sg:want P3]
The second radio button should read "Check for updates, but let me choose whether to download and install them" like in Windows (7) update settings.
(In reply to sdrocking from comment #42)
> The second radio button should read "Check for updates, but let me choose
> whether to download and install them" like in Windows (7) update settings.

Would seem the comment 16 attachment didn't include the wording change that the attachment comment mentioned.
(Assignee)

Comment 44

6 years ago
Indeed. Please file a followup-bug if you think that needs to be changed.
Created Bug 697385
Depends on: 697385
(Assignee)

Updated

6 years ago
Depends on: 701987
You need to log in before you can comment on or make changes to this bug.