2013725 - "Account Settings" in "New email" window does not bring these settings on the foreground

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[Clément Lefèvre]

When clicking "Account Settings" in any of the two menus it's available (see bug 2013723) in the "New email" window, it opens these settings in a tab of the main window. But it behaves differently compared to "Settings", "Address Book" and some others: these menu entries while opening a dedicated tab on the main window also brings that main window to the foreground so that the user can see what they requested. "Account settings" doesn't, which can lead the user to think the menu is broken and nothing happened if the "New Email" window is maximized/in fullscreen.

Bug reported from Thunderbird on Linux/Wayland. macOS while having this entry in a different menu is also affected. Can't tell about Windows, and I can only guess Linux on Xorg is affected too, can't check right now.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: Bug 2013725 - "Account Settings" from compose window should focus the settings window. r=#thunderbird-reviewers

Comment 2

[Clément Lefèvre]

Just to understand, the other similar I reported almost simultaneously had status for 147 set to wontfix and 148 to affected. What's the difference between affected and wontfix here when 149 is the target in both cases?

Surprised it's already too late for 148 on small patches like that too!

Comment 3

[Magnus Melin [:mkmelin]]

That would mostly be about developer intentions. Perhaps indeed we could try to get this into 148.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/057a348fe983
"Account Settings" from compose window should focus the settings window. r=arschmitz

Comment 5

[Francesco]

In the compose window, I observe this:

1. Tools > Address book: Brings main window forward, and opens AB tab -> good
2. Tools > Settings: Brings main window forward, opens the compose settings tab -> good
3. Tools > Account Settings: Does not bring main window forward, but opens the Account settings tab -> not fixed

Looks like the bug wasn't fixed. Looking a the code change, point 2. was fixed, but not point 3.

Am I missing something?

Comment 6

[Clément Lefèvre]

(In reply to Francesco from comment #5)

In the compose window, I observe this:

1. Tools > Address book: Brings main window forward, and opens AB tab -> good
2. Tools > Settings: Brings main window forward, opens the compose settings tab -> good
3. Tools > Account Settings: Does not bring main window forward, but opens the Account settings tab -> not fixed

Looks like the bug wasn't fixed. Looking a the code change, point 2. was fixed, but not point 3.

Am I missing something?

Point two is already good on current stable, it wasn't fixed from this patch. So did the patch fixed something in this case?

Comment 7

[Francesco]

(In reply to Clément Lefèvre from comment #6)

Point two is already good on current stable, it wasn't fixed from this patch.

Oh, yes, the code is the same:
https://searchfox.org/comm-central/rev/1b8c9d317208ac8ec9795bee945099322b9389c0/mail/base/content/mailCore.js#576

So did the patch fixed something in this case?

Maybe not.

Comment 8

[Clément Lefèvre]

(In reply to Francesco from comment #7)

(In reply to Clément Lefèvre from comment #6)

Point two is already good on current stable, it wasn't fixed from this patch.

Oh, yes, the code is the same:
https://searchfox.org/comm-central/rev/1b8c9d317208ac8ec9795bee945099322b9389c0/mail/base/content/mailCore.js#576

So did the patch fixed something in this case?

Maybe not.

Did you test a nightly build/build from master? Afaik it's not advanced beyond nighties yet.

Comment 9

[Hartmut Welpmann [:welpy-cw]]

(In reply to Francesco from comment #5)

In the compose window, I observe this:

1. Tools > Address book: Brings main window forward, and opens AB tab -> good
2. Tools > Settings: Brings main window forward, opens the compose settings tab -> good
3. Tools > Account Settings: Does not bring main window forward, but opens the Account settings tab -> not fixed

Looks like the bug wasn't fixed. Looking a the code change, point 2. was fixed, but not point 3.

Point 2 wasn't broken in the first place, as I see it, replacing openOptionsDialog by openPreferencesTab is essentially a no-op.

To fix point 3 (this bug), just adding win.focus(); here should be enough.

By the way, regarding point 1 without a 3-pane window already open, see bug 2017004.

Comment 10

[Clément Lefèvre]

Point 2 wasn't broken in the first place, as I see it, replacing openOptionsDialog by openPreferencesTab is essentially a no-op.

I'm probably missing something but… what's the point of such a function then? Why not replace every occurrence of openOptionsDialog by openPreferencesTab in the code?

(As it's compiled code, it probably doesn't make any difference once compiled, but well…)

Comment 11

[Magnus Melin [:mkmelin]]

Sorry, I'm not sure what I saw. Will clean up and fix it properly.

Comment 12

[Magnus Melin [:mkmelin]]

Attachment: Bug 2013725 - Take2: "Account Settings" in "New email" window does not bring these settings on the foreground. r=arschmitz

Remove the pointlessly duplicated openOptionsDialog.

Comment 13

[Clément Lefèvre]

(In reply to Magnus Melin [:mkmelin] from comment #12)

Created attachment 9546461 [details]
Bug 2013725 - Take2: "Account Settings" in "New email" window does not bring these settings on the foreground. r=arschmitz

Remove the pointlessly duplicated openOptionsDialog.

I think I get the choice back then, as it used to be an Options dialog, but was changed years ago to a Preferences tab. I guess when the tab was introduced it was maybe progressively? In the end, as it's today a Preferences tab, wouldn't it make sense to keep the latter rather than the former?

Comment 14

[Magnus Melin [:mkmelin]]

Right, I'm keeping openPreferencesTab

Comment 15

[Francesco]

Just curious, comment 9 suggested adding a focus() call. I can't see that in the patch, instead, a focus() call is even removed.

Comment 16

[Magnus Melin [:mkmelin]]

Strange, I was sure I included that. Going back to fix it my editor has the change already. Updating the patch now

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1ff1b1080005
Take2: "Account Settings" in "New email" window does not bring these settings on the foreground. r=arschmitz