Account Manager: Convert the dialogs to subdialogs
Categories
(Thunderbird :: Account Manager, task)
Tracking
(Not tracked)
People
(Reporter: Paenglab, Assigned: Paenglab)
References
Details
Attachments
(1 file, 5 obsolete files)
(deleted),
patch
|
Paenglab
:
review+
|
Details | Diff | Splinter Review |
Like the prefs dialogs we should convert the Account Manager dialogs to in-content subdialogs.
Assignee | ||
Comment 1•5 years ago
|
||
This is a WIP. Only the global return receipts, the two certificate dialogs in "Security" and the "Edit SMTP Server" dialogs are converted.
I have the problem, that the dialogs don't stop the execution of the calling JS until they are closed. This makes that the calling doesn't know when the dialog is closed and the content should be reloaded when something has changed. I tried similar to https://searchfox.org/comm-central/source/mail/components/preferences/general.js#1032 with a closing callback but it seems it doesn't work because I need a parent.gSubDialog.open() to to call the dialog in the iframe.
For example, when I create a new SMTP config with "Add...", the config is created but the listbox doesn't update to show the new entry.
Alessandro, do you have maybe a solution for this issue? Then I can try this with the other dialogs.
Updated•5 years ago
|
Comment 2•5 years ago
|
||
I tried updating the editSMTPServer()
method to trigger a callback once the dialog is closed, and it seemed to work.
parent.gSubDialog.open(
"chrome://messenger/content/SmtpServerEdit.xhtml",
null,
args,
testCallback
);
function testCallback() {
console.log("callback");
}
Was this what you were trying to do?
Another approach we could try is to add observers to those lists and select elements in order to be able to listen to any change and ignore this whole subdialog callback thingy.
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
(In reply to Alessandro Castellani (:aleca) from comment #2)
Was this what you were trying to do?
Yes, it was this. I tried it on am-main.js with opening the am-identities-list.xhtml. I'll try the SmtpServerEdit.xhtml this evening.
Assignee | ||
Comment 4•5 years ago
|
||
This works now, except two dialogs:
- The certificate picker in Security "Select..." because they are opened through a C++ file https://searchfox.org/comm-central/source/mailnews/extensions/smime/src/nsCertPicker.cpp#257
- The
Edit Identity
dialog because I don't know how to open a second subdialog from a subdialog. This should work: https://searchfox.org/comm-central/source/mail/components/preferences/subdialogs.js#10. I Also attach a follow-up patch which moves the identities list inside the main pane. This makes it able to open the Identities dialog in a subdialog. But this one needs still a solution for opening subdialogs from it.
Alessandro, Magnus, please can you show me with a small example how to open a subdialog from a subdialog? We could use this in the prefs for the Edit LDAP directories dialog.
Assignee | ||
Comment 5•5 years ago
|
||
Assignee | ||
Comment 6•5 years ago
|
||
This is the follow-up patch on top of the first patch to move the identities dialog to the main pane. This makes it possible to directly open the Edit Identity dialog as subdialog. But this dialog has other buttons to open additional dialog which needs a solution (this doesn't work actually). I need the solution to open a subdialog from a subdialog.
Alessandro and Magnus, what do you think about this solution? If you think this would get a go, please check if the new description above the identities list would be okay like this.
Comment 7•5 years ago
|
||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #7)
Comment on attachment 9126951 [details] [diff] [review]
1615417-identities-in-main.patchReview of attachment 9126951 [details] [diff] [review]:
::: mail/locales/en-US/chrome/messenger/am-identities-list.dtd
@@ +1,5 @@<!-- This Source Code Form is subject to the terms of the Mozilla Public
- License, v. 2.0. If a copy of the MPL was not distributed with this
- file, You can obtain one at http://mozilla.org/MPL/2.0/. -->+<!ENTITY identitiesListDesc.label "Manage the identities for this account. The first identity is used by default and showed above.">
localization key must be changed so localizers can pick up
Entity changed from identitiesListManageDesc.label to identitiesListDesc.label.
::: mail/locales/en-US/chrome/messenger/am-identity-edit.dtd
@@ +4,5 @@<!-- LOCALIZATION NOTE (identityDialog.style): This value should be roughly
equal to the value of accountManager.size entity minus the value
of accountTree.width entity. -->
+<!ENTITY identityDialog.size "width: 75ch; height: 56em;">this key would also need to be bumped
Also this changed from identityDialog.style to identityDialog.size.
Assignee | ||
Comment 10•5 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #8)
Comment on attachment 9126949 [details] [diff] [review]
1615417-AM-subdialog.patchReview of attachment 9126949 [details] [diff] [review]:
All in all, I don't quite like how the identities are now listed at the
bottom of the main account page.
I think it all needs to be rethought. Now the information goes in all
directions: first vertical in the tree, then when editing horizontally in
tabs.
Then the account main page is really (almost) the same as what is in an
identity dialog.
This patch is for the status quo but only with in-content subdialogs. The other was as proposal for the identities list.
Do you have an answer for how I need to call a subdialog from a subdialog (https://searchfox.org/comm-central/source/mail/components/preferences/subdialogs.js#10)? Then the other patch isn't needed as it is.
Comment 11•5 years ago
|
||
Sorry I don't know.
Assignee | ||
Comment 12•5 years ago
|
||
Nothing special needed for subdialog from subdialog. :-)
This patch changes only that the dialogs open in subdialogs instead separate dialogs. Only the "Cert Picker" and the "LDAP directory edit" dialogs don't open in-contnet because they are invoked through C++.
No special casing for Thunderbird now. SM has to follow with in-content dialogs.
Comment 13•5 years ago
|
||
Updated•5 years ago
|
Comment 14•5 years ago
|
||
There's an error found by lint too.
Assignee | ||
Comment 15•5 years ago
|
||
Review comments and ESLint failures fixed. I have some test failures: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&selectedJob=289336055.
It looks like the test wants to open the dialog as window, but now it's in-content.
Geoff, Magnus, please could you look what is needed to fix this errors? I don't know this mochitests not at all.
Comment 16•5 years ago
|
||
I suspect that's not a test failure, but in the archive options something's off with gIdentity
[task 2020-02-18T12:24:08.906Z] 12:24:08 INFO - Console message: [JavaScript Error: "TypeError: can't access property "archiveGranularity", gIdentity is undefined" {file: "chrome://messenger/content/am-archiveoptions.js" line: 19}]
Assignee | ||
Comment 17•5 years ago
|
||
I thought it has something to do with this: https://searchfox.org/comm-central/source/mail/test/browser/account/browser_archiveOptions.js#106 that it waits for a window that never appears.
Comment 18•5 years ago
|
||
Assignee | ||
Comment 19•5 years ago
|
||
Thank you Geoff, tests passing now https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ff56aceda6693759f6a60f2d0b31fe958a81aa39
Assignee | ||
Updated•5 years ago
|
Comment 20•5 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/1c9f4fdf7772
Implement the subdialogs in Account Manager. r=mkmelin DONTBUILD
Updated•5 years ago
|
Description
•