360891 - Account manager "read receipts" policy should link to global option.

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, enhancement)

Description

[Philip Chalmers]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: version 1.5.0.8 (20061025)

I installed T-bird and launched it. I was immediately asked to define an account. The account dialogue includes a section which allows the user to follow the global "read receipt" policy or define an account-specific policy. As a new user:
* I had no idea what the current global policy was.
* It took me a long time to find it, since it's buried quite deep in the Tools->Options dialogue.

I suggest T-bird's developers should:
* Walk through the whole "first launch" process, check for other items which are set in other dialogues and provide copies of and preferably links to the setting dialogues for these items.
* Make this standard procedure for new versions of T-bird.

Reproducible: Always

Steps to Reproduce:
1. Install new completely instance of T-bird.
2. While defining 1st account, try to find out what the global "read receipt" policy is.
3.

Actual Results:  
See description

Expected Results:  
Show (as read-only boxes) the current global "read receipt" policy in the "read receipt" policy section of the account dialogue.
Link to the relevant Options dialogue pane.

Comment 1

[Mike Cowperthwaite]

The way you described the problem, I thought you were encountering the receipts settings during the New Account Wizard.  This may comes as a surprise to you, but most users don't bother looking at every setting in the account -- that's kind of an "expert" behavior, and experts tend to be able to discover options on their own, even when deeply buried.

That said, there are a number of per-account settings which have related settings in Options, where it would be nice to provide some sort of linkage between the two.

Comment 2

[:aceman]

I'll check if it is possible to have button in the account manager opening the preferences window.

Comment 3

[:aceman]

This is shaping up nicely, bug 718139 enables the infrastructure to open a specific tab in Preferences dialog.

However, as the account manager is in /mailnews I am not sure how to access the openOptionsDialog() from mail/base/content/mailCore.js.

I could make this work for Thunderbird:
    let win = Services.wm.getMostRecentWindow("mail:3pane");
    win.openOptionsDialog("paneAdvanced", "generalTab");

Is this acceptable for TB?

Then, what about Seamonkey? Is there something I could all there? Or should I skip this functionality for SM? How? Using preprocessing of the am-mdn.xul file?

Comment 4

[:aceman]

Attachment: patch for TB

This is the proposal for TB. To see the full functionality, this must be applied on top of patch in bug 718139. But it can also be tried standalone.

Ian, what about Seamonkey?

Comment 5

[Ian Neal]

Comment on attachment 631694 [details] [diff] [review]
patch for TB

>+++ b/mailnews/extensions/mdn/content/am-mdn.js
>+/**
>+ * Opens Preferences (Options) dialog on the Advanced pane, General tab
>+ * so that the user sees where the global receipts settings can be found.
>+ */
>+function showGlobalReceipts() {
>+  try {
>+    let win = Services.wm.getMostRecentWindow("mail:3pane");
>+    win.openOptionsDialog("paneAdvanced", "generalTab");
>+  } catch (e) {
>+    Components.utils.reportError(e);
>+  }
>+}
Obviously you will need a way of working out if you are calling to a TB or SM preferences window and so use the appropriate function (goPreferences for SM).

>+++ b/mailnews/extensions/mdn/content/am-mdn.xul
>+      <hbox id="prefChoices" align="center" flex="1">
>         <radiogroup id="identity.use_custom_prefs" wsm_persist="true" genericattr="true"
>                     preftype="bool" prefstring="mail.identity.%identitykey%.use_custom_prefs"
>+                    oncommand="EnableDisableCustomSettings();" flex="1">
>+          <hbox align="center" flex="1">
>+            <radio id="identity.select_global_prefs" value="false"
>+                   label="&useGlobalPrefs.label;"
>+                   accesskey="&useGlobalPrefs.accesskey;"/>
>+            <spacer flex="1"/>
>+            <button label="&globalReceipts.label;"
>+                    accesskey="&globalReceipts.accesskey;"
>+                    oncommand="showGlobalReceipts();"/>
>+          </hbox>
>+          <radio id="identity.select_custom_prefs" value="true"
>+                 label="&useCustomPrefs.label;"
>                  accesskey="&useCustomPrefs.accesskey;"/>
Nit: Where you are changing things do one attribute per line if it does not fit on a single line (so hbox element is fine).
>         </radiogroup>
I would say SM would want this but worth checking if Karsten.

Comment 6

[:aceman]

(In reply to Ian Neal from comment #5)
> >+function showGlobalReceipts() {
> >+  try {
> >+    let win = Services.wm.getMostRecentWindow("mail:3pane");
> >+    win.openOptionsDialog("paneAdvanced", "generalTab");
> >+  } catch (e) {
> >+    Components.utils.reportError(e);
> >+  }
> >+}
> Obviously you will need a way of working out if you are calling to a TB or
> SM preferences window and so use the appropriate function (goPreferences for
> SM).

That is what I ask. Maybe SM's window is not "mail:3pane" so I could distinguish by that. Or file preprocessing (ifdef MOZ_THUNDERBIRD).
It looks like SM has receipts on a separate pane "receipts_pane" so it could be called via goPreferences.

Comment 7

[:aceman]

It looks SM's window is also "mail:3pane". Maybe I can just check for the existence of the function (openOptionsDialog vs goPreferences).

Comment 8

[Ian Neal]

(In reply to :aceman from comment #7)
> It looks SM's window is also "mail:3pane". Maybe I can just check for the
> existence of the function (openOptionsDialog vs goPreferences).

Yes, sounds like a good route.

Comment 9

[:aceman]

Attachment: patch v2

So this contains also the SM version. Please test it.

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 631699 [details] [diff] [review]
patch v2

I think I might prefer "global return receipt preferences" to be a link, but I'm going to say ui-r=me for this version, too, in case you don't want to implement it.  ;)

Oh, it would also be nice if it took us straight into the Read Receipts dialog, but I'm guessing that would be "hard", so I'm not going to block on that either.

Thanks,
Blake.

Comment 11

[:aceman]

A link would be some a UI element not yet used :) But that seems good as this really is a new feature (not an AM subdialog). I'll check it out.
I think I could activate the "Read Receipts" subdialog. I already had to implement the tab switching first.

Comment 12

[Karsten Düsterloh]

Comment on attachment 631699 [details] [diff] [review]
patch v2

Nice!

>+            <radio id="identity.select_global_prefs"
>+                   value="false"
>+                   label="&useGlobalPrefs.label;"
>+                   accesskey="&useGlobalPrefs.accesskey;"/>
>+            <spacer flex="1"/>
>+            <button label="&globalReceipts.label;"
>+                    accesskey="&globalReceipts.accesskey;"
>+                    oncommand="showGlobalReceipts();"/>

I'd suggest adding an id for the button as well, just in case.

Comment 13

[:aceman]

mnyromyr, would you be also OK with a link instead of a button?

Comment 14

[:aceman]

Bwinton, a link is not easy, the text shown as the "Use my global return receipt preferences for this account" is a label attribute of the <radio> and is highlighted on mouse over. I could produce some <label>s to create the link and surrounding text. But it seems I would loose the connection to the <radio> and the hovering effect...

Comment 15

[Ian Neal]

Comment on attachment 631699 [details] [diff] [review]
patch v2

>+function showGlobalReceipts() {
>+  let win = Services.wm.getMostRecentWindow("mail:3pane");
>+
>+  // If openOptionsDialog() exists, we are in Thunderbird.
>+  if (typeof win.openOptionsDialog == "function")
>+    win.openOptionsDialog("paneAdvanced", "generalTab");
>+  // If goPreferences() exists, we are in Seamonkey.
>+  if (typeof win.goPreferences == "function")
>+    win.goPreferences("receipts_pane");
>+}
What happens if there is not mail:3pane window open (e.g. just a compose or single message window open)?
Probably need to do something like:
let win = Services.wm.getMostRecentWindow("mail:3pane") ||
          Services.wm.getMostRecentWindow("mail:messageWindow") ||
          Services.wm.getMostRecentWindow("msgcompose");

Null check of win is possibly needed too.
f- for the moment

Comment 16

[:aceman]

Attachment: patch v3

I have kept the button, I could figure out the link. But this new version opens the subdialog of Receipts Options and does not even show the main Preferences dialog.

I am not yet sure if the handling of this subdialog opening should be done universally in the preferences.js or just in advanced.js as it is now.

Comment 17

[:aceman]

...I could NOT figure out the link...

Comment 18

[:aceman]

Attachment: patch v3

Sorry, last patch had a typo.
The SM part has no UI changes, only fixes from Ian so review can be done in parallel to the TB ui-review.

Comment 19

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 635041 [details] [diff] [review]
patch v3

On the Mac, it only shows the Advanced >> General pane, and not the Return Receipts sub-dialog, but other than that, it seems fine…  (mconley might be able to help you debug that part.)

ui-r=me with that fixed!

Thanks,
Blake.

Comment 20

[:aceman]

Comment on attachment 635041 [details] [diff] [review]
patch v3

Maybe you get something in the error console?

Comment 21

[Blake Winton (:bwinton) (:☕️)]

Nothing that looks related.

Comment 22

[Ian Neal]

Comment on attachment 635041 [details] [diff] [review]
patch v3

For SeaMonkey the button needs to be on a new line as with the current patch it causes scrollbars to appear.
The button label itself should probably be "Global Preferences…" rather than "Global preferences…" as that is the format for the majority of other button labels.

Comment 23

[Ian Neal]

(In reply to Ian Neal from comment #22)
> Comment on attachment 635041 [details] [diff] [review]
> patch v3
> 
> For SeaMonkey the button needs to be on a new line as with the current patch
> it causes scrollbars to appear.
The other option would to be wrap the label for the radio button so it doesn't cause the scrollbar to appear

Comment 24

[:aceman]

Ok, I'll try to move the button down but still aligned to the right.

Comment 25

[:aceman]

Attachment: patch v4 - Only subdialog shown, Pref dialog flashes at closing.

Comment 26

[:aceman]

Attachment: patch v4.1 - Pref dialog + subdialog shown

Another version according to mconley's complaints. This one does not hide the Preferences dialog, it opens it normally and opens the subdialog on top of it. It also moves the subdialog opening to preferences.js to make it a general feature (it will be used for other bugs too).

Comment 27

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 644410 [details] [diff] [review]
patch v4 - Only subdialog shown, Pref dialog flashes at closing.

Nope, too weird looking on Mac.

Comment 28

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 644488 [details] [diff] [review]
patch v4.1 - Pref dialog + subdialog shown

Yeah, this is much better.  ui-r=me!

Thanks,
Blake.

Comment 29

[Ian Neal]

Comment on attachment 644488 [details] [diff] [review]
patch v4.1 - Pref dialog + subdialog shown

>+++ b/mailnews/base/prefs/content/amUtils.js

>+function openPrefsFromAccountManager(aTBPaneId, aTBTabId, aTBOtherArgs, aSMPaneId) {
>+  let win = Services.wm.getMostRecentWindow("mail:3pane") ||
>+            Services.wm.getMostRecentWindow("mail:messageWindow") ||
>+            Services.wm.getMostRecentWindow("msgcompose");;
Nit: double ;

r=me with that fixed.

Comment 30

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 644488 [details] [diff] [review]
patch v4.1 - Pref dialog + subdialog shown

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

aceman:

Besides the ;; that Ian caught earlier, I just have a >80 char complaint. Other than that, this looks good, and behaves as expected.

Thanks!

-Mike

::: mailnews/extensions/mdn/content/am-mdn.js
@@ +151,5 @@
> + * Opens Preferences (Options) dialog on the pane and tab where
> + * the global receipts settings can be found.
> + */
> +function showGlobalReceipts() {
> +  openPrefsFromAccountManager("paneAdvanced", "generalTab", {subdialog: "showReturnReceipts"}, "receipts_pane");

Let's break this up over two lines.

::: mailnews/extensions/mdn/content/am-mdn.xul
@@ +38,5 @@
> +            <spacer flex="1"/>
> +            <button id="globalReceiptsLink"
> +                    label="&globalReceipts.label;"
> +                    accesskey="&globalReceipts.accesskey;"
> +                    oncommand="showGlobalReceipts();"/>

Normally, I'd bug you to use the command pattern here, instead of using oncommand...but I'll let it slide here - this patch has been waiting long enough.

Comment 31

[:aceman]

(In reply to Mike Conley (:mconley) from comment #30)
> Normally, I'd bug you to use the command pattern here, instead of using
> oncommand...but I'll let it slide here - this patch has been waiting long
> enough.

There is no usage of 'command' in whole Account manager. So I think new commands should not be introduced in random patches. You could file a bug for converting AM globally and assign it to me :)

Comment 32

[:aceman]

Attachment: patch v5

Thanks!

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/5f15e0bad5a0

Comment 34

[:aceman]

Oh guys, what about disabling the button when the "use global preferences" radio option is not selected? In the same way the custom settings are disabled.