Last Comment Bug 757823 - Make Account Provisioner order-form content tab handle and display links
: Make Account Provisioner order-form content tab handle and display links
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Account Manager (show other bugs)
: 13 Branch
: x86 All
: -- normal (vote)
: Thunderbird 15.0
Assigned To: Mike Conley (:mconley) - (Away until June 29th)
:
Mentors:
Depends on:
Blocks: AccountProvisioner 758237
  Show dependency treegraph
 
Reported: 2012-05-23 06:21 PDT by Mike Conley (:mconley) - (Away until June 29th)
Modified: 2012-05-28 14:06 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
+
fixed


Attachments
Patch v1 (14.77 KB, patch)
2012-05-24 08:49 PDT, Mike Conley (:mconley) - (Away until June 29th)
bwinton: review+
bwinton: ui‑review+
Details | Diff | Review
Patch v2 - carrying forward ui-r+ and r+ from bwinton (14.67 KB, patch)
2012-05-25 11:02 PDT, Mike Conley (:mconley) - (Away until June 29th)
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Review

Description Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 06:21:24 PDT
STR:

1) Go to File > New > Get a New Mail Account
2) Make sure Hover.com is selected, type in a name, and click "Search"
3) Choose to purchase one of the suggested accounts
4) In the order form tab that opens, try clicking on the "Billing" link

What happens?

The link opens in the default browser, without the session cookie.

What's expected?

We want to allow the user to open certain links from within Thunderbird, since their session cookie is needed in order for the link to do its job.


From Pete Brevin at the Hover team:

"...links in our HTML always open in a new browser window (without the Thunderbird session cookie).  We'd like a way to make some of them, such as "return to billing info" or "cancel this order", be handled by Thunderbird, otherwise the user experience is horribly disconnected."

Blake - attaching a click handler shouldn't be too hard, but where do we draw a distinction between which links to open in content, and which to open in browser?  Or should we just have a blanket policy of opening URLs within the provider domain within content, like webSearchTab?
Comment 1 Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 06:57:43 PDT
So here are the workarounds that Blake and I brainstormed:

1. Pass the session token as a URL parameter, to allow the session to persist when opening links in the default browser. This onus would be almost entirely on our providers - the time and effort required is undefined.

2. Open every link in Thunderbird. This is simple to do, but introduces some UX issues. Our providers will likely want to streamline their order forms so that they act more like wizards, providing their own back/forward mechanisms (since TB lacks any obvious ones), and not branching off of the main flow.

3. The providers give us a list of RegEx's that we check each clicked link against to determine whether or not to open them in content, or in the default browser. This would require effort on Sancus's part to get the RegEx's passed back, and would require effort to modify our provisioner tabs to check against the RegEx's.
Comment 2 Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 07:47:08 PDT
A few additional comments for each workaround:

Workaround 1 (the session token is passed as a URL parameter):

I'm unsure of the value of this workaround, since it's not clear to me exactly *what* URLs would need to do this, and what function they'd serve. And, once outside of Thunderbird, there's no way to go back without the user explicitly choosing to go back themselves. This means that the end-goal of the returned XML is not easily obtained if the user is suddenly branched over to their default browser.

Workaround 2 (Have Thunderbird open each link within the content tab):

Like I stated before, this will require some modifications on our providers' part to make this work correctly. We definitely don't want the user getting lost deep within the bowels of a providers website, without easy mechanisms for them to get back to their primary task of ordering their account.

From this perspective, I think opening links should be considered "expensive", and that things like inline-popups / tooltips are more desirable than full-blown page loads.

But again, this puts a great deal of onus on our providers to get this work done, and in a short amount of time.

Workaround 3 (Have Thunderbird check against a series of RegEx's):

For this, providers would have to do an audit of their order forms and generate RegEx's for the links that need to be opened by TB (or perhaps the other way around - generate RegEx's for links that need to be opened in the default browser. It really makes no difference to me.)

Those RegEx's would need to go to Sancus, who will then need to pass those back when the provider list is loaded in the Account Provisioner dialog. Those RegEx's will need to be passed to the order form tab, and then we'll need to do some magic to make the tab check against those RegEx's to make its decision.
Comment 3 Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 07:48:47 PDT
I'm leaning towards Workaround 3 (Have Thunderbird check against a series of RegEx's), but I'm completely open to suggestions / feedback.

Or are there other solutions that we haven't come up with?
Comment 4 Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 08:06:13 PDT
Just got mail from one of our providers saying they prefer Workaround 2 (Have Thunderbird open each link within the content tab).

"We providers need to work on optimizing the process, keeping the user focused on it and finalizing it painlessly. We also thought about the user getting lost in sub-pages and we started to adapt the site to open links in a new window if they're not part of the process during a thunderbird session.

I see you worked on option 3, that would be great too, but the regexps are a little difficult to build as we send the customer to the bank's website for payment and they handle it the way they want there...Building the list of all possible websites out there, or managing the bank changes to its own website sounds difficult."

That's good feedback, and if providers are willing to put in the work to optimize / smooth the experience on their forms, it's making me lean towards Workaround 2.
Comment 5 Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 09:53:42 PDT
There's also been a request to make it possible for providers to optionally open certain links in the default browser, either by capturing links to be opened in new windows, or via anchor nodes with a special class that TB recognizes.
Comment 6 Yann Brelière 2012-05-23 10:02:36 PDT
If Thunderbird opened links within the content tab, a 'target' attribute on a link wouldn't already make the link open in the browser ?
Comment 7 Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 10:11:32 PDT
Yann:

If Thunderbird chooses to keep opening links within itself (like in the web search tabs, for example), links with the target attribute set to "_blank" open up in Thunderbird as new content tabs.

-Mike
Comment 8 Yann Brelière 2012-05-23 10:23:11 PDT
Oh, ok! Thanks.
Yes, it could be nice, then. But the most important is that I could open a link in a new tab (of the browser or of Thunderbird), while every other link stays on the main Thunderbird tab.
"capturing links to be opened in new windows" could be nice, but only if we mark them as such with a specific class or target: we need our javascript popups (like the bill) to open in Thunderbird with the current session.
Comment 9 Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 10:38:21 PDT
(In reply to Yann Brelière from comment #8)
> Oh, ok! Thanks.
> Yes, it could be nice, then. But the most important is that I could open a
> link in a new tab (of the browser or of Thunderbird), while every other link
> stays on the main Thunderbird tab.
> "capturing links to be opened in new windows" could be nice, but only if we
> mark them as such with a specific class or target: we need our javascript
> popups (like the bill) to open in Thunderbird with the current session.

Ah, ok, I see.  So in that case, I think marking anchors with a special class to indicate that they should open in the default browser is probably where we want to go then.  And we just default to opening everything else in the current content tab, or a new content tab (in the event of target="_blank").

Is that sufficient?
Comment 10 Pete Bevin 2012-05-23 10:39:51 PDT
Another option:

4. Support <a target="_self" href="..."> to force load inside the same Thunderbird window, but other links would open in the browser (this has the advantage of being compatible with a browser view of the same content, but probably doesn't help with links on a bank's web site as per comment #4).

My preference is for option 2: open all links in the same Thunderbird window.  As a provider, it's in my interest as well as Mozilla's to make sure users don't get lost inside my web site, and that is completely under my control.

I strongly dislike option 1 - it doesn't solve one of my current bugs, which is that I have a link to "cancel this order" which returns the cancellation XML response that only Thunderbird recognizes.  And option 3 requires me to maintain a URL list externally to my web site, which is going to be error-prone.
Comment 11 Yann Brelière 2012-05-23 10:51:26 PDT
(In reply to Mike Conley (:mconley) from comment #9)
> Ah, ok, I see.  So in that case, I think marking anchors with a special
> class to indicate that they should open in the default browser is probably
> where we want to go then.  And we just default to opening everything else in
> the current content tab, or a new content tab (in the event of
> target="_blank").
> 
> Is that sufficient?

Yes, that would be best.
And what about using a named target instead of a class, to be more explicit?


(In reply to Pete Bevin from comment #10)
> My preference is for option 2: open all links in the same Thunderbird
> window.  As a provider, it's in my interest as well as Mozilla's to make
> sure users don't get lost inside my web site, and that is completely under
> my control.

Yes, I agree.
Comment 12 Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 10:54:13 PDT
Alright, sounds like we're taking workaround #2.  Cool.

Now one last question before I start working on this:

How important is it to kick out to the default browser, as opposed to a new content tab in Thunderbird? Is one preferred over the other, or is there a need for both abilities?

The advantages of opening a new content tab in Thunderbird:

1. If the user hasn't got their browser running, this will almost certainly be faster to load and display.
2. Less application context switching. Sure, the context has changed - but they're still within Thunderbird, and closing the tab keeps them in Thunderbird.

Disadvantages to opening a new content tab in Thunderbird:

1. Lack of basic browser capbilities, like back/forward navigation, the URL bar, bookmarking, etc.



If we go with opening content tabs in Thunderbird, target="_blank" will do the job, and we're done.

If we go with opening content tabs in the default browser, Thunderbird can just treat links with target="_blank" as links to open in the default browser.

If we go with allowing providers to choose either, we can perhaps introduce a new special target type, like "_external" that Thunderbird can listen for, for opening links in the default browser.

Preferences?
Comment 13 Mike Conley (:mconley) - (Away until June 29th) 2012-05-23 11:00:17 PDT
(In reply to Yann Brelière from comment #11)
> 
> Yes, that would be best.
> And what about using a named target instead of a class, to be more explicit?
> 

Excellent idea - yes, a named target makes much more sense, thanks.
Comment 14 Yann Brelière 2012-05-23 11:06:52 PDT
(In reply to Mike Conley (:mconley) from comment #12)
> If we go with opening content tabs in Thunderbird, target="_blank" will do
> the job, and we're done.
> 
> If we go with opening content tabs in the default browser, Thunderbird can
> just treat links with target="_blank" as links to open in the default
> browser.
> 
> If we go with allowing providers to choose either, we can perhaps introduce
> a new special target type, like "_external" that Thunderbird can listen for,
> for opening links in the default browser.
> 
> Preferences?

The minimum would be that links open in the content tab, and target="_blank" opens in a Thunderbird tab.
Opening all target="_blank" links in the browser could be nice, but it shouldn't prevent opening javascript popups in Thunderbird.
So... either only opening links with a target in the browser (and javascript links in thunderbird), or using a specific target (option 3) to open in the browser, and everything else treated like the first option.
Comment 15 Mike Conley (:mconley) - (Away until June 29th) 2012-05-24 08:42:53 PDT
Ok, so here's what I've gotten from that:

1)  Links by default open in the order form tab.
2)  Links with target="_blank" open in the default browser.
3)  Links opened with "window.open" open the link in a new content tab in Thunderbird.

Is this acceptable?
Comment 16 Yann Brelière 2012-05-24 08:45:41 PDT
That's perfect! :)
Would this be available in the first release of the account provisioner?
Comment 17 Mike Conley (:mconley) - (Away until June 29th) 2012-05-24 08:47:19 PDT
(In reply to Yann Brelière from comment #16)
> That's perfect! :)
> Would this be available in the first release of the account provisioner?

I have a patch ready that's baking on our try server right now. In a few hours it'll be ready, and I'll post the link to the builds here for you all to try.

If you're happy with the behaviour, I'll put the patch up for review and we can try to land this in beta in time for the TB 13 release.
Comment 18 Mike Conley (:mconley) - (Away until June 29th) 2012-05-24 08:49:20 PDT
Created attachment 626828 [details] [diff] [review]
Patch v1

Actually, I might as well put the patch up now - at the very least to checkpoint the work.
Comment 19 Mike Conley (:mconley) - (Away until June 29th) 2012-05-24 11:28:43 PDT
Try builds for Windows, OSX and Linux are available here:

https://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/mconley@mozilla.com-f8fe361f4e71/

Pete / Yann:  Please confirm that the behaviour matches your expectations, and I'll request review on the patch.
Comment 20 Pascal Bouchareine 2012-05-25 01:55:19 PDT
Wonderful! From what I could test, the navigation is ok, extra bank hrefs stay in the tab, extra info links open in blank tbird tabs. I even managed to finalize a mailbox creation hacking the provider list and got a congratulation message.

It seems ok to me, Yann is off until next tuesday so if Pete confirms the navigation/target= behaviour you discussed is ok, that's a go for me!
Comment 21 Mike Conley (:mconley) - (Away until June 29th) 2012-05-25 06:47:30 PDT
Comment on attachment 626828 [details] [diff] [review]
Patch v1

Got the green-light from our providers with this behaviour, so requesting review.
Comment 22 Blake Winton (:bwinton) (:☕️) (PTO 'til London. Find me there for quick answers!) 2012-05-25 08:23:25 PDT
Comment on attachment 626828 [details] [diff] [review]
Patch v1

Grah!  I had reviewed this, and then Firefox lost it!

Anyways, tl;dr - ui-r=me.

>+++ b/mail/test/mozmill/newmailaccount/test-newmailaccount.js
>@@ -893,16 +855,135 @@ function test_other_lang_link_hides() {
>+/**
>+ * Test that clicking on links in the order form open in the same account
>+ * provisioner tab.
>+ */
>+function test_internal_link_opening_behaviour() {
>+  const kTargetPath = "/target.html";
[snip]
>+  // Click on the internal link.
>+  let internal = doc.getElementById("internal");
>+  mc.click(new elib.Elem(internal));

I think we can inline the "internal" variable without loss of readability.

>+  // We should load the target page in the current tab browser.
>+  wait_for_browser_load(tab.browser, function(aURL) {
>+    return aURL.host == "localhost" && aURL.path == kTargetPath;

Ditto the kTargetPath.  (Unless you want to share it between two functions.)

>+++ b/mail/test/mozmill/shared-modules/test-content-tab-helpers.js
>@@ -47,35 +47,42 @@ var utils = {};
>-const MODULE_REQUIRES = ['folder-display-helpers', 'window-helpers'];
>+const MODULE_REQUIRES = ['folder-display-helpers',
>+                         'window-helpers',
>+                         'mock-object-helpers',];

Why the extra comma here?  I thought those were only useful if you put the ]; on the next line…

So those are all pretty small things, so I'm going to say r=me, with those fixed.

Thanks,
Blake.
Comment 23 Mike Conley (:mconley) - (Away until June 29th) 2012-05-25 11:02:45 PDT
Created attachment 627294 [details] [diff] [review]
Patch v2 - carrying forward ui-r+ and r+ from bwinton

We want this for shipment of AP in TB 13.
Comment 24 Mike Conley (:mconley) - (Away until June 29th) 2012-05-28 09:52:00 PDT
comm-central: https://hg.mozilla.org/comm-central/rev/dae4853973c3
Comment 25 Mike Conley (:mconley) - (Away until June 29th) 2012-05-28 14:00:12 PDT
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/a3c081242e75
Comment 26 Mike Conley (:mconley) - (Away until June 29th) 2012-05-28 14:06:51 PDT
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/e893db8a2ea5

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