735240 - Filelink: Failures when account creation is cancelled

Status: VERIFIED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Thomas D. (:thomas8)]

STR

on today's Trunk

1 compose
2 add big attachment
3 [review] without any bigfiles accounts defined, click "Link" from bigfiles alert
4 don't proceed with bigfiles account creation (I watched the dropbox video and then somehow exited)
5 context menu from big attachment > convert to >
6 click on blank menu item

actual result

after 5:

"context menu > convert to" has this (confusing, something wrong here):

o regular attachment
----------------------
[blank menu item]

after 6:

clicking on blank menu item attempts conversion into linked attachment, which subsequently fails miserably in all sorts of ways:
- confusing messages of application trying to connect to dropbox or something
- after exiting from that message (still without creating or having any bigfiles account):
- spinner on big attachments keeps turning forever, pretends to be "uploading" (while I don't have anything to upload to)
- convert to is disabled
- cannot send the message (send disabled)
- creating a new message with same attachment no longer shows the bigfiles alert

Expected result

Pls ensure that after cancellation of bigfiles account creation, the context menu is behaving better, and that user can't get into undefined attachment states (I'll leave the details to you)

Comment 1

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #0)
> 1 compose
> 2 add big attachment
> 3 [details] [diff] [review] without any bigfiles accounts defined, click

...and can someone pls file a bug against bugzilla about not parsing enumerations that happen to have a link word at the end of one list item (like attachment, bug, etc.) as wrong links... omg

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Thomas:

Does this problem persist with the latest EarlyBird?  I believe you were experiencing bug 735668.

-Mike

Comment 3

[Thomas D. (:thomas8)]

(In reply to Mike Conley (:mconley) from comment #2)
> Does this problem persist with the latest EarlyBird?

Yes, most of it (just tested with TB 14.0a1 2012-03-15)

> I believe you were experiencing bug 735668.

Maybe, or maybe not, but that bug alone didn't fix this, at all.
The bad part is that we create a useless "" account which in turn messes the whole behaviour up. Deja vu from Mail Account Setup Wizzard.

STR (Updated, better)

no filelink accounts set up yet (ensure from tools > options > attachments > outgoing that you don't have broken filelink accounts hanging round)

1 compose
2 add big attachment .
3 without any bigfiles accounts defined, click "Link" from bigfiles alert
4 from "Set up filelink" dialogue, choose "Dropbox", then click "Set up account" button (just that, proceed with 5)
5 don't proceed with dropbox account creation: just close the Dropbox Sign In browser window without signing in and without creating a new account
6 from "Set up filelink" dialogue (still open behind the now-closed dropbox browser window), choose "Cancel"
7 Send mail using Ctrl+Enter
7b close dropbox browser login dialogue without logging in or creating acct
8 Realize you'll wait forever for your attachment to upload to nowhere, and try to fix it to send your message anyway:
a) attachment context menu > cancel upload
b) attachment context menu > convert to (correctly disabled)
c) close draft, save

Actual results

after STR #...

4 on the "Set up filelink" dialogue, we display "Checking authorization..." - are we really?

5 after user has closed dropbox browser window, we still show "Checking authorization..." with spinner - now we definitely aren't checking any more, and I suppose we can know that because our dropbox browser window is gone (closed by user) without sending any signals...
And: There's no point of keeping "Set up filelink" dialogue if the only button which is enabled is "Cancel" - either allow me to start with "Set up account" again, or just get the dialogue out of the way when I close the Dropbox browser window without creating an account.

6 After "Cancel": Everything should now be back "to normal", and that's how it *looks*: attachment looks like normal attachment (no spinner), shows real attachment size, and the filelink info pane at the bottom is gone (OT: what's the technical term for these bars again, like attachment reminder and this one...?). BUT *under the hood*, we got it all wrong...

7 Upon "Send", the seemingly "normal" attachment turns out to be a wannabe linkfile: 
- Spinner starts, "Uploading to ..." [sic] and we actually tell the truth because we are "Uploading to [nowhere]..." and user can wait forever with nothing happening (remember, user clicked "Link", but did *not* create a dropbox account, so there is still *no* filelink account, so there's nowhere we can upload to!)
- Dropbox browser window opens

7b after closing dropbox browser window without logging in or creating acct
- Sent button is (correctly) disabled
- User is stuck, because:

8a) attachment's "Cancel upload" does *nothing*: spinner still spins, still uploading to nowhere...
b) convert to is disabled, so nowhere to go:
User *cannot* send this message.
c) closing draft with "save" unexpectedly closed TB application (reproducible, somewhat erratically) - the bird just flies away... (And I'd probably do the same in its position, after all what it had to endure from these big new extra features marching in with new bugs while old bugs in basic functionality are still languishing with no roadmap to fix them...)

Expected result:

- do not create an empty dropbox filelink account if user exits from account creation without creating an account
- probably do not consider the attachment a linked bigfile attachment after user has exited account creation triggered from "Link" button without creating a filelink account (especially if there's no filelink account at all)
- get the behaviour right so that no matter which roads the user decides to take or leave, TB is still functional as it was before trying the new feature
- don't kill the bird on normal use
- pls figure out the details yourself (STR provided)

Comment 4

[Thomas D. (:thomas8)]

Attachment: Screenshot of useless empty dropbox account created by STR of comment 3

Comment 5

[David :Bienvenu]

yes, I saw that too - if the account creation fails, we need to cleanup the empty account (in particular, the display name is empty, I believe)

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

It looks like the "cancelled" function was attempting to call "connectFailureCallback" from the wrong function.

With this patch, closing the OAuth window causes the new Filelink account window to report that something has gone wrong.  Everything else seems to operate as expected, and we don't have ghostly accounts cropping up all over the place.

I've also put an additional check into the preferences dialog to ensure that we got a valid accountKey back when creating an account.

Comment 7

[David :Bienvenu]

Comment on attachment 608362 [details] [diff] [review]
Patch v1

looks good

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Committed to comm-central as http://hg.mozilla.org/comm-central/rev/1d12ed82aaf4
Committed to comm-aurora as http://hg.mozilla.org/releases/comm-aurora/rev/e7c441b31724

Comment 9

[Thomas D. (:thomas8)]

I've tried everything possible to trigger this bug again, but it's truly gone.
Thanks Mike, great job. I wish we could fix some of the old bugs at the same speed.

The only problem which I still see is 8c, a regression where TB dies when closing draft window with red [x] button: Bug 738907. Could this be caused by the changes for bigfiles?