363700 - Software Installation (pref-smartupdate.xul) prefs page is designed for xpfe SM not suiterunner

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Mark Banner (:standard8)]

The Software Installation page (http://lxr.mozilla.org/seamonkey/source/suite/common/pref/pref-smartupdate.xul) is designed for setting the prefs that were to set up update notifications for SeaMonkey as an xpfe application.

With suiterunner we're intending to use the toolkit update service for the app and extensions. We need to write a prefs page for updates specific to suiterunner - or rewrite the existing one.

Note that at the moment automatic updates are disabled by default in suiterunner - I guess we'd need to talk to MoCo/MoFo to find out what we need to do to a) let automatic updates take place b) host the facilities for automatic updates.

Comment 1

[Mark Banner (:standard8)]

When the smart update page is redone, we need to also ensure we remove the following prefs from browser-prefs.js (version in suite/browser) as they apply to xpfe version only:

update_notifications.enabled
update_notifications.provider.0.frequency
update_notifications.provider.0.datasource

Comment 2

[Mark Banner (:standard8)]

Attachment: Proposed re-layout

I'd like to propose this as the rough new layout of the Software Installation pane (also known as smartupdate). Looking at the available prefs (http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/suite/browser/browser-prefs.js&rev=1.57&mark=208-316#208) I think this is all we need.

Comment 3

[[e-mail]]

Comment on attachment 251465 [details]
Proposed re-layout

You can't duplicate those radiobutton accesskeys!

It'd be nice to get that extensions.dss.enabled pref into the debug panel ;-)

Comment 4

[Mark Banner (:standard8)]

(In reply to comment #3)
> (From update of attachment 251465 [details])
> You can't duplicate those radiobutton accesskeys!
Opps, that's a copy and paste error

> It'd be nice to get that extensions.dss.enabled pref into the debug panel ;-)

Yep, should be easy to do.

Comment 5

[Robert Kaiser]

A button to directly launch Add-Ons Manager would probably be a good idea to fit in there.

Comment 6

[Mark Banner (:standard8)]

Attachment: Patch v1

First version of the patch. Application options are disabled in the xul (with note) until we sort out arrangements for software update (probably sometime after we get suiterunner going on trunk).

The extension options work and set prefs as they should - I've also checked that an extension update (in this case it was chatzilla) is detected and I was notified on next start up (providing I hadn't gone into add-on manager which would have indicated it anyway).

Comment 7

[Mark Banner (:standard8)]

Attachment: Revised screenshot

Comment 8

[Mark Banner (:standard8)]

Attachment: Patch v1a

This patch just removes a change that shouldn't have been there.

Comment 9

[Ian Neal]

Comment on attachment 252046 [details] [diff] [review]
Patch v1a

>Index: suite/locales/en-US/chrome/common/pref/pref-smartupdate.dtd
>===================================================================
>@@ -6,6 +6,21 @@
> <!ENTITY allowedSites.label                   "Allowed Sites">
> <!ENTITY allowedSites.accesskey               "S">
> 
>+<!-- These entities are only used in suiterunner (MOZ_XUL_APP=1 set) -->
>+<!ENTITY autoUpdates.label                    "Automatically check for updates to:">
>+<!ENTITY appUpdates.label                     "&brandShortName;">
>+<!ENTITY appUpdates.accesskey                 "y">
>+<!ENTITY appDaily.accesskey                   "d">
>+<!ENTITY appWeekly.accesskey                  "w">
>+<!ENTITY extensionsUpdates.label              "Installed Add-ons">
>+<!ENTITY extensionsUpdates.accesskey          "n">
>+<!ENTITY extensionsDaily.accesskey            "i">
>+<!ENTITY extensionsWeekly.accesskey           "k">
>+<!ENTITY appUpdatesDisabled.desc              "Automatic updates to &brandShortName; are not currently available. This feature is intended for implementation before the next release.">
>+<!ENTITY addonManagerButton.label             "Add-on Manager">
>+<!ENTITY addonManagerButton.accesskey         "g">
>+
>+<!-- These entities are only used in xpfe SeaMonkey (MOZ_XUL_APP not set) -->
> <!ENTITY enableNotification.label             "Notify me when new and updated software is available">
> <!ENTITY enableNotification.accesskey         "N">
> <!ENTITY requireConfirmation.label            "Require manual confirmation of each install">
>@@ -18,7 +33,11 @@
> <!ENTITY updateNotifications.desc             "Check for new versions of this software.  No information about you or your computer will be revealed during this operation.">
> <!ENTITY enableUN.label                       "Check for updates:">
> <!ENTITY enableUN.accesskey                   "C">
>-<!ENTITY weekly.label                         "weekly">
>-<!ENTITY weekly.accesskey                     "w">
> <!ENTITY monthly.label                        "monthly">
> <!ENTITY monthly.accesskey                    "m">
>+
>+<!-- These entites are used for both types of SeaMonkey -->
>+<!ENTITY daily.label                          "daily">
>+<!ENTITY daily.accesskey                      "d">
>+<!ENTITY weekly.label                         "weekly">
>+<!ENTITY weekly.accesskey                     "w">
You do not use daily.accesskey at all, weekly.accesskey is only used in xpfe SM and daily.label is only used in suiterunner. So you need to re-arrange/remove to make things correct.
r=me with those changes.

Comment 10

[Mark Banner (:standard8)]

Attachment: Patch v2

This patch is similar to v1a bug I'm planning on applying it at the time of the change over on trunk, and therefore I've removed all the ifdefs and sorted out the locale file.

Comment 11

[[e-mail]]

Comment on attachment 264498 [details] [diff] [review]
Patch v2

>+          <!-- disabled until SM updates are available -->
>+          <radiogroup id="appUpdateFrequency" orient="horizontal" 
>+                      prefstring="app.update.interval"
>+                      prefdefval="86400">
>+            <radio id="appFreqDaily" label="&daily.label;" disabled="true"
>+                   accesskey="&appDaily.accesskey;" value="86400"/>
>+            <radio id="appFreqWeekly" label="&weekly.label;" disabled="true"
>+                   accesskey="&appWeekly.accesskey;" value="604800"/>
>+          </radiogroup>
I believe that these days you can put disabled="true" on the radiogroup instead of having to put it on both the radios individually.

Comment 12

[Ian Neal]

Comment on attachment 264498 [details] [diff] [review]
Patch v2

>Index: suite/locales/en-US/chrome/common/pref/pref-smartupdate.dtd
>===================================================================
I would look at using "e" as the accesskey for "Allow web sites..." which would free up "A" to be used below.
> <!ENTITY allowedSites.label                   "Allowed Sites">
> <!ENTITY allowedSites.accesskey               "S">
Possibly change to "A"

>+<!ENTITY autoUpdates.label                    "Automatically check for updates to:">
>+<!ENTITY appUpdates.label                     "&brandShortName;">
>+<!ENTITY appUpdates.accesskey                 "y">
Possibly change to "S" using letters like "y" or "g" is not good.

>+<!ENTITY addonManagerButton.label             "Add-on Manager">
>+<!ENTITY addonManagerButton.accesskey         "g">
Possibly change to "M" for same reason as above.

r=me with those changes discussed.

Comment 13

[Mark Banner (:standard8)]

Attachment: Patch for checkin

Addresses Ian's comments. Ready for checkin at switchover to suiterunner (when bug 328887 is resolved).

Comment 14

[Mark Banner (:standard8)]

Moving this bug from blocking bug 328887 to depending on it as we're now not going to check this in until that bug is implemented.

Comment 15

[Mark Banner (:standard8)]

Patch checked in -> fixed.