Last Comment Bug 600505 - App update preference ui favors disabling app update
: App update preference ui favors disabling app update
Status: RESOLVED FIXED
[sg:want P3]
: polish, user-doc-needed, ux-jargon, ux-tone
Product: Firefox
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: Firefox 10
Assigned To: Steffen Wilberg
:
Mentors:
Depends on: 697385 701987
Blocks: 600500
  Show dependency treegraph
 
Reported: 2010-09-29 04:13 PDT by Robert Strong [:rstrong] (use needinfo to contact me)
Modified: 2013-11-13 02:20 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
screenshot (71.38 KB, image/png)
2011-06-22 16:13 PDT, Steffen Wilberg
faaborg: ui‑review+
Details
patch (17.88 KB, patch)
2011-06-22 16:18 PDT, Steffen Wilberg
no flags Details | Diff | Review
patch v1 (21.60 KB, patch)
2011-07-29 16:23 PDT, Steffen Wilberg
no flags Details | Diff | Review
patch v1.1 (21.55 KB, patch)
2011-08-05 12:05 PDT, Steffen Wilberg
no flags Details | Diff | Review
patch v1.2 (21.55 KB, patch)
2011-08-25 14:08 PDT, Steffen Wilberg
no flags Details | Diff | Review
patch v1.3 (21.55 KB, patch)
2011-08-25 14:16 PDT, Steffen Wilberg
robert.strong.bugs: review-
Details | Diff | Review
patch v1.4 (22.26 KB, patch)
2011-09-27 15:55 PDT, Steffen Wilberg
no flags Details | Diff | Review
Linux screenshot of patch v1.4 (69.54 KB, image/png)
2011-09-27 15:59 PDT, Steffen Wilberg
faaborg: ui‑review+
Details
Windows screenshot of patch v1.5 (21.60 KB, text/plain)
2011-10-01 02:20 PDT, Steffen Wilberg
no flags Details
patch v1.5 (22.33 KB, patch)
2011-10-01 02:35 PDT, Steffen Wilberg
no flags Details | Diff | Review
Windows screenshot of patch v1.5 (41.03 KB, image/png)
2011-10-01 02:43 PDT, Steffen Wilberg
faaborg: ui‑review+
Details
patch v1.6 (22.35 KB, patch)
2011-10-04 14:05 PDT, Steffen Wilberg
robert.strong.bugs: review+
Details | Diff | Review

Description Robert Strong [:rstrong] (use needinfo to contact me) 2010-09-29 04:13:29 PDT
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)
Comment 1 Robert Strong [:rstrong] (use needinfo to contact me) 2010-09-29 04:14:31 PDT
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
Comment 2 Mike Beltzner [:beltzner, not reading bugmail] 2010-09-29 08:16:27 PDT
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.
Comment 3 Alex Faaborg [:faaborg] (Firefox UX) 2010-09-29 23:35:51 PDT
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.
Comment 4 Alex Limi (:limi) — Firefox UX Team 2011-05-31 16:33:46 PDT
(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.
Comment 5 Steffen Wilberg 2011-06-22 03:25:31 PDT
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?
Comment 6 Robert Strong [:rstrong] (use needinfo to contact me) 2011-06-22 10:40:04 PDT
In comment #0 I wasn't stating that we should have the exact same options as Windows. It was only an example for UX.
Comment 7 Steffen Wilberg 2011-06-22 16:13:38 PDT
Created attachment 541216 [details]
screenshot

Robert: too late, you already had me hooked :)
Comment 8 Steffen Wilberg 2011-06-22 16:18:04 PDT
Created attachment 541219 [details] [diff] [review]
patch
Comment 9 Tony 2011-06-23 10:08:31 PDT
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
Comment 10 Robert Strong [:rstrong] (use needinfo to contact me) 2011-06-23 10:39:44 PDT
(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 11 Alex Faaborg [:faaborg] (Firefox UX) 2011-07-28 17:04:37 PDT
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)
Comment 12 Steffen Wilberg 2011-07-29 16:23:59 PDT
Created attachment 549507 [details] [diff] [review]
patch v1

With strings from comment 11, and more comments.
Comment 13 Steffen Wilberg 2011-08-05 12:05:18 PDT
Created attachment 551108 [details] [diff] [review]
patch v1.1

Unbitrotted, and fixed double accesskey.
Comment 14 Steffen Wilberg 2011-08-21 07:08:38 PDT
updateManual.label has a typo ("not recommend").
Comment 15 Steffen Wilberg 2011-08-25 14:08:11 PDT
Created attachment 555833 [details] [diff] [review]
patch v1.2

Fixed that typo.
Gavin, should I pick another reviewer?
Comment 16 Steffen Wilberg 2011-08-25 14:16:25 PDT
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)
Comment 17 Steffen Wilberg 2011-09-08 14:14:30 PDT
https://hg.mozilla.org/projects/ux/rev/74008670f6ff
Comment 18 Steffen Wilberg 2011-09-19 14:33:43 PDT
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...
Comment 19 Steffen Wilberg 2011-09-24 08:01:25 PDT
Oops, patch v1.2 and 1.3 are identical.
Will update updateCheck.label per bug 600500 comment 14 in the next iteration.
Comment 20 Robert Strong [:rstrong] (use needinfo to contact me) 2011-09-26 15:54:15 PDT
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.
Comment 21 Alex Limi (:limi) — Firefox UX Team 2011-09-26 22:48:15 PDT
(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. :)
Comment 22 Steffen Wilberg 2011-09-27 15:55:55 PDT
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.
Comment 23 Steffen Wilberg 2011-09-27 15:59:32 PDT
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.
Comment 24 Steffen Wilberg 2011-09-27 16:06:53 PDT
https://hg.mozilla.org/projects/ux/rev/dce8c0b89e94
Comment 25 Steffen Wilberg 2011-10-01 02:20:05 PDT
Created attachment 563946 [details]
Windows screenshot of patch v1.5
Comment 26 Steffen Wilberg 2011-10-01 02:35:11 PDT
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.
Comment 27 Robert Strong [:rstrong] (use needinfo to contact me) 2011-10-01 02:37:21 PDT
Comment on attachment 563946 [details]
Windows screenshot of patch v1.5

This is the patch and not the Windows screenshot
Comment 28 Steffen Wilberg 2011-10-01 02:43:24 PDT
Created attachment 563948 [details]
Windows screenshot of patch v1.5

Oops.
Comment 29 Robert Strong [:rstrong] (use needinfo to contact me) 2011-10-01 02:48:38 PDT
Comment on attachment 563948 [details]
Windows screenshot of patch v1.5

Looks good!
Comment 30 Alex Faaborg [:faaborg] (Firefox UX) 2011-10-03 15:17:11 PDT
Comment on attachment 562898 [details]
Linux screenshot of patch v1.4

looks good
Comment 31 Alex Faaborg [:faaborg] (Firefox UX) 2011-10-03 15:17:29 PDT
Comment on attachment 563948 [details]
Windows screenshot of patch v1.5

looks good
Comment 32 Jeff Grossman 2011-10-03 20:37:26 PDT
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".
Comment 33 Steffen Wilberg 2011-10-04 00:12:03 PDT
(In reply to Jeff Grossman from comment #32)
That's a typo, thanks for catching that!
Comment 34 Steffen Wilberg 2011-10-04 14:05:01 PDT
Created attachment 564662 [details] [diff] [review]
patch v1.6

Typo fixed.
Comment 35 Robert Strong [:rstrong] (use needinfo to contact me) 2011-10-05 12:46:03 PDT
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
Comment 36 Steffen Wilberg 2011-10-05 12:51:48 PDT
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.
Comment 37 Steffen Wilberg 2011-10-11 12:04:17 PDT
(In reply to myself from comment #36)
Things not to tell your reviewer:
#1: "take all the time you need" ;-)
Comment 38 Robert Strong [:rstrong] (use needinfo to contact me) 2011-10-11 12:07:06 PDT
I promise, I'm close to being done though I was close last week as well. ;)
Comment 39 Robert Strong [:rstrong] (use needinfo to contact me) 2011-10-12 15:07:09 PDT
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
Comment 41 Ed Morley [:emorley] 2011-10-14 03:51:03 PDT
https://hg.mozilla.org/mozilla-central/rev/2d9076dbcd15
Comment 42 Siddhartha Dugar [:sdrocking] 2011-10-25 04:04:27 PDT
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.
Comment 43 Ed Morley [:emorley] 2011-10-25 04:13:43 PDT
(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.
Comment 44 Steffen Wilberg 2011-10-25 05:19:19 PDT
Indeed. Please file a followup-bug if you think that needs to be changed.
Comment 45 Siddhartha Dugar [:sdrocking] 2011-10-26 02:12:24 PDT
Created Bug 697385

Note You need to log in before you can comment on or make changes to this bug.