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

Status: RESOLVED FIXED

(Thunderbird :: FileLink, defect)

Description

[Jorg K (CEST = GMT+2)]

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

Comment 1

[Jorg K (CEST = GMT+2)]

Aceman, do you have time for this?

Comment 2

[Patrick Cloke [:clokep]]

Did we break something in bug 1045845 that caused this?

Comment 3

[Patrick Cloke [:clokep]]

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?

Comment 4

[Jorg K (CEST = GMT+2)]

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.

Comment 5

[Jorg K (CEST = GMT+2)]

Looks like Patrick will take care of it.

Comment 6

[Patrick Cloke [:clokep]]

(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.

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: 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.

Comment 8

[Jorg K (CEST = GMT+2)]

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 9

[:aceman]

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?

Comment 10

[Jorg K (CEST = GMT+2)]

Aceman, you have time to fix this?

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

Comment 11

[:aceman]

Attachment: alternative patch

Comment 12

[Patrick Cloke [: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!

Comment 13

[Jorg K (CEST = GMT+2)]

I tested the cancel/onUnInit bit. onOK hasn't changed.

Comment 14

[:aceman]

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).

Comment 15

[Jorg K (CEST = GMT+2)]

(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.

Comment 16

[:aceman]

https://hg.mozilla.org/comm-central/rev/f3332dc78843