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

RESOLVED FIXED in Thunderbird 45.0

Status

Thunderbird
FileLink
--
major
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Jorg K (GMT+2), Assigned: aceman)

Tracking

Trunk
Thunderbird 45.0

Thunderbird Tracking Flags

(thunderbird45+ fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Set up Filelink modal dialog can't be cancelled leaving the session unusable.
(Reporter)

Updated

2 years ago
Severity: normal → critical
(Reporter)

Comment 1

2 years ago
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?
(Reporter)

Comment 4

2 years ago
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.
(Reporter)

Comment 5

2 years ago
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.
(Reporter)

Comment 7

2 years ago
Created attachment 8697330 [details] [diff] [review]
Works with this.

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)
(Reporter)

Comment 8

2 years ago
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;"

Updated

2 years ago
tracking-thunderbird45: --- → +
(Assignee)

Comment 9

2 years ago
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-
(Reporter)

Comment 10

2 years ago
Aceman, you have time to fix this?

My other suggestion was change
ondialogcancel="return addAccountDialog.onUnInit();"
to
ondialogcancel="addAccountDialog.onUnInit();return 0;"
(Assignee)

Comment 11

2 years ago
Created attachment 8697404 [details] [diff] [review]
alternative patch
Attachment #8697404 - Flags: review?(philipp)
(Reporter)

Updated

2 years ago
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
(Reporter)

Comment 13

2 years ago
I tested the cancel/onUnInit bit. onOK hasn't changed.
Keywords: checkin-needed
(Assignee)

Comment 14

2 years ago
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).
(Reporter)

Comment 15

2 years ago
(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.
(Assignee)

Comment 16

2 years ago
https://hg.mozilla.org/comm-central/rev/f3332dc78843
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0

Updated

2 years ago
status-thunderbird45: --- → fixed
You need to log in before you can comment on or make changes to this bug.