Closed Bug 1231654 Opened 4 years ago Closed 4 years ago

Set up Filelink modal dialog can't be cancelled leaving the session unusable

Categories

(Thunderbird :: FileLink, defect, major)

defect
Not set
major

Tracking

(thunderbird45+ fixed)

RESOLVED FIXED
Thunderbird 45.0
Tracking Status
thunderbird45 + fixed

People

(Reporter: jorgk, Assigned: aceman)

References

Details

Attachments

(1 file, 1 obsolete file)

Set up Filelink modal dialog can't be cancelled leaving the session unusable.
Severity: normal → critical
Aceman, do you have time for this?
Flags: needinfo?(acelists)
Did we break something in bug 1045845 that caused this?
Severity: critical → major
Do you see any errors in the error console when this happens? What kind of FileLink account are you creating? Can you give more detailed steps to reproduce?
STR, dead easy:

Tools > Options, Attachments, Outgoing, Add.
Set up Filelink appears.

You're dead, you can't cancel this panel.

First time I added an account to get rid of it, second time I couldn't add another account, so I had to kill the process, which I don't like doing in production. Grrrrr.

The Box stuff works nicely, but this panel kills you.
Looks like Patrick will take care of it.
Flags: needinfo?(acelists)
(In reply to Jorg K (GMT+1) from comment #5)
> Looks like Patrick will take care of it.

I don't have time to look at this right now. I can do a review.

To be clear, I wonder if one of the changes in https://hg.mozilla.org/comm-central/rev/c73ce0b62910#l1.1 is to blame.
Attached patch Works with this. (obsolete) — Splinter Review
I have no idea what's going on here. This is the function that runs behind the cancel button. If it returns 'false', the panel isn't dismissed. So I changed it to 'true' and the panel goes away.
Attachment #8697330 - Flags: review?(clokep)
Alternatively change:
https://dxr.mozilla.org/comm-central/source/mail/components/cloudfile/content/addAccountDialog.xul#23
from
ondialogcancel="return addAccountDialog.onUnInit();"
to
ondialogcancel="addAccountDialog.onUnInit();return 0;"
Comment on attachment 8697330 [details] [diff] [review]
Works with this.

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

While this should solve the closing problem, it probably creates another one that now the onOK function would return true, which is opposite to what the linked patch does and what was there before it.

It looks like clicking the "set up account" (OK) button should not close the dialog, because it then opens a new Authorize dialog, while keeping the old dialog open.
Can keep onOK returning false?
Attachment #8697330 - Flags: feedback-
Aceman, you have time to fix this?

My other suggestion was change
ondialogcancel="return addAccountDialog.onUnInit();"
to
ondialogcancel="addAccountDialog.onUnInit();return 0;"
Attachment #8697404 - Flags: review?(philipp)
Attachment #8697330 - Attachment is obsolete: true
Attachment #8697330 - Flags: review?(clokep)
Comment on attachment 8697404 [details] [diff] [review]
alternative patch

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

Assuming this was tested, it looks reasonable to me. Thanks for fixing this!
Attachment #8697404 - Flags: review?(philipp) → review+
Assignee: nobody → acelists
Blocks: 1045845
Status: NEW → ASSIGNED
I tested the cancel/onUnInit bit. onOK hasn't changed.
Keywords: checkin-needed
I changed onOK to return false, i.e. the same value as before the change to onUnInit. So the only effect of this patch should be the change in onUnInit which now allows the dialog to close on Cancel.
Of course I tested both buttons (Cancel and Set up account).
(In reply to :aceman from comment #14)
> which now allows the dialog to close on Cancel.
Who didn't test this in the first place, grrrrr. A modal dialog that can't be closed.
I had to kill my session.
https://hg.mozilla.org/comm-central/rev/f3332dc78843
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0
You need to log in before you can comment on or make changes to this bug.