Closed Bug 1617449 Opened 6 months ago Closed 5 months ago

Make the "Remove Account" dialog in-content

Categories

(Thunderbird :: Account Manager, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 76.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(2 files, 2 obsolete files)

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

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

Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9128374 - Flags: review?(mkmelin+mozilla)

Found a white space that slipped in.

Attachment #9128374 - Attachment is obsolete: true
Attachment #9128374 - Flags: review?(mkmelin+mozilla)
Attachment #9128375 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9128375 [details] [diff] [review]
1617449-remove-account-in-content.patch

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

LGTM, r=mkmelin
Attachment #9128375 - Flags: review?(mkmelin+mozilla) → review+

Going to land. Fixed a linting error.

Target Milestone: --- → Thunderbird 75.0

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/2a7bb79c9728
Make the "Remove Account" dialog in-content. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED

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

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

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?

Flags: needinfo?(geoff)

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

Flags: needinfo?(geoff)
Attachment #9129642 - Flags: review?(richard.marti)
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.
Attachment #9129642 - Flags: review?(richard.marti) → review?(mkmelin+mozilla)

Same patch that was reviewed but with the linting fix.

Attachment #9128375 - Attachment is obsolete: true
Attachment #9129808 - Flags: review+
Comment on attachment 9129642 [details] [diff] [review]
1617449-remove-account-tests-1.diff

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

LGTM, r=mkmelin
Attachment #9129642 - Flags: review?(mkmelin+mozilla) → review+

Pushed by geoff@darktrojan.net:
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

Status: REOPENED → RESOLVED
Closed: 6 months ago5 months ago
Resolution: --- → FIXED
Target Milestone: Thunderbird 75.0 → Thunderbird 76.0
You need to log in before you can comment on or make changes to this bug.