1617449 - Make the "Remove Account" dialog in-content

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, task)

Description

[Richard Marti (:Paenglab)]

The "remove Account" dialog is actually a normal dialog. We can convert it to a in-content subdialog too.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1617449-remove-account-in-content.patch

I tried it with the French language XPI which normally uses longer strings and it still shows all correctly

Comment 2

[Richard Marti (:Paenglab)]

Attachment: 1617449-remove-account-in-content.patch

Found a white space that slipped in.

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9128375 [details] [diff] [review]
1617449-remove-account-in-content.patch

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

LGTM, r=mkmelin

Comment 4

[Magnus Melin [:mkmelin]]

Going to land. Fixed a linting error.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2a7bb79c9728
Make the "Remove Account" dialog in-content. r=mkmelin

Comment 6

[Magnus Melin [:mkmelin]]

Backed out due to test failures: https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedJob=290214886

Backed out changeset 2a7bb79c9728 (bug 1617449) for test failures in comm/mail/test/browser/account/browser_deletion.js, comm/mail/test/browser/account/browser_settingsInfrastructure.js and comm/mail/test/browser/account/browser_tree.js
https://hg.mozilla.org/comm-central/rev/68a089e630441b952c3a583f19686a8c17c32758

Comment 7

[Richard Marti (:Paenglab)]

I tried to use a normal dialog for the tests: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=40842642021b3283e6f509895422ef7e1cf33296 it looks already a bit better but I know too less to fix the remaining issues.

Geoff, please could you fix me this tests?

Comment 8

[Geoff Lankow (:darktrojan)]

Attachment: 1617449-remove-account-tests-1.diff

I've changed the test to use the in-content dialog.

Using a real dialog like you tried wouldn't have worked because the main page is waiting for a response (from a window it opened, but you opened the window) before updating itself. Also when opening the real dialog you passed some bogus arguments to it, which doesn't help. :-)

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b99b28f2d74b318b7ee729c843791dbcdade1cef

Comment 9

[Richard Marti (:Paenglab)]

Comment on attachment 9129642 [details] [diff] [review]
1617449-remove-account-tests-1.diff

Better Magnus checks this as I know not enough about tests to review it.

Comment 10

[Richard Marti (:Paenglab)]

Attachment: 1617449-remove-account-in-content.patch

Same patch that was reviewed but with the linting fix.

Comment 11

[Magnus Melin [:mkmelin]]

Comment on attachment 9129642 [details] [diff] [review]
1617449-remove-account-tests-1.diff

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

LGTM, r=mkmelin

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9c310e265b9d
Make the "Remove Account" dialog in-content. r=mkmelin
https://hg.mozilla.org/comm-central/rev/2169c9897abf
Convert remove_account tests to use in-content dialog. r=mkmelin