Last Comment Bug 738234 - Account provisioner success window blocks compose window password prompt
: Account provisioner success window blocks compose window password prompt
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Account Manager (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Thunderbird 15.0
Assigned To: Mike Conley (:mconley) - (needinfo me!)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-22 06:34 PDT by Mike Conley (:mconley) - (needinfo me!)
Modified: 2012-05-25 07:08 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
-
+
fixed
fixed


Attachments
Patch v1 (973 bytes, patch)
2012-05-22 10:46 PDT, Mike Conley (:mconley) - (needinfo me!)
bwinton: ui‑review-
Details | Diff | Review
Make success dialog non-modal - Patch v1 (1.47 KB, patch)
2012-05-23 08:45 PDT, Mike Conley (:mconley) - (needinfo me!)
bwinton: review+
bwinton: ui‑review+
Details | Diff | Review
Patch v2 (fixes tests - carrying forward ui-r+/r+ from bwinton) (3.00 KB, patch)
2012-05-23 12:42 PDT, Mike Conley (:mconley) - (needinfo me!)
mozilla: approval‑comm‑aurora+
mozilla: approval‑comm‑beta+
Details | Diff | Review

Description Mike Conley (:mconley) - (needinfo me!) 2012-03-22 06:34:43 PDT
STR:

1)  Order a new email address from Hover.com through the account provisioner
2)  When the success window appears, choose to compose a new message.
3)  Write a message to some recipient, and send

What happens?

The sending is blocked until the success window is closed.  This is because the composer is attempting to prompt for a password, but is being blocked by the success modal window.
Comment 1 Mark Banner (:standard8) 2012-04-13 07:56:02 PDT
Moving to 13, as we're not shipping account prov in 12.
Comment 2 Mike Conley (:mconley) - (needinfo me!) 2012-05-22 10:46:39 PDT
Created attachment 626084 [details] [diff] [review]
Patch v1

So I know that we'd ideally like to have our providers echo back the user's password via XML so that the user doesn't have to re-enter the password for their new account...

...but in the event that this doesn't happen, this patch closes the success dialog when the user chooses to write a new message, which eliminates the password-prompt problem.

Blake - sound like the right approach?
Comment 3 Blake Winton (:bwinton) (:☕️) 2012-05-22 13:09:02 PDT
Comment on attachment 626084 [details] [diff] [review]
Patch v1

Ugh, given that there are more things we would want the user to be able to do after writing email, I don't think this is really the right approach, unless we can re-open the success dialog after the compose window closes…

/me mumbles something about compose-in-a-tab and tab-modal password prompts…

I mean, I guess this is better than hanging, so ui-r=me if we really need to land something, but it also doesn't feel like the right solution…
Comment 4 Mike Conley (:mconley) - (needinfo me!) 2012-05-23 07:10:56 PDT
Blake:

How about we just make the success dialog non-modal?

-Mike
Comment 5 Mike Conley (:mconley) - (needinfo me!) 2012-05-23 08:45:38 PDT
Created attachment 626466 [details] [diff] [review]
Make success dialog non-modal - Patch v1

Making the success dialog non-modal is both possible, and does seem to address the issue (in that the password prompt successfully came up when I tried to send mail from a new Hover.com account when using a composer spawned from the success dialog).

What do you think of this approach, Blake?
Comment 6 Blake Winton (:bwinton) (:☕️) 2012-05-23 10:03:09 PDT
Comment on attachment 626466 [details] [diff] [review]
Make success dialog non-modal - Patch v1

Yeah, I think this works…

ui-r=me!

And the code looks good too, so let's say r=me while we're here to save time.

Thanks,
Blake.
Comment 7 Mike Conley (:mconley) - (needinfo me!) 2012-05-23 10:12:25 PDT
Comment on attachment 626466 [details] [diff] [review]
Make success dialog non-modal - Patch v1

I think this is pretty low risk.
Comment 8 Mike Conley (:mconley) - (needinfo me!) 2012-05-23 12:42:06 PDT
Created attachment 626557 [details] [diff] [review]
Patch v2 (fixes tests - carrying forward ui-r+/r+ from bwinton)

Whoops - this broke one of our newmailaccount Mozmill tests, since it was using the modal window tools instead of our normal window tools.

This patch fixes that, and our newmailaccount tests pass with this patch.  Carrying forward ui-r+ and r+ from bwinton, since the change is test-only.
Comment 10 Mark Banner (:standard8) 2012-05-25 01:07:02 PDT
(In reply to Mike Conley (:mconley) from comment #9)
> comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/5fa5eaf77d48

That was actually landed on a relbranch, so I've transplanted it to the default branch:

https://hg.mozilla.org/releases/comm-beta/rev/383e1aa8a1c5
Comment 11 Mike Conley (:mconley) - (needinfo me!) 2012-05-25 07:08:41 PDT
(In reply to Mark Banner (:standard8) from comment #10)
> (In reply to Mike Conley (:mconley) from comment #9)
> > comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/5fa5eaf77d48
> 
> That was actually landed on a relbranch, so I've transplanted it to the
> default branch:
> 
> https://hg.mozilla.org/releases/comm-beta/rev/383e1aa8a1c5

Yikes, sorry about that. Thanks Mark.

Note You need to log in before you can comment on or make changes to this bug.