Last Comment Bug 752666 - provide explicit mechanism to specify URL load target
: provide explicit mechanism to specify URL load target
Status: VERIFIED FIXED
[blocking-webrtdesktop1+], [appreview...
: dev-doc-complete
Product: Firefox Graveyard
Classification: Graveyard
Component: Webapp Runtime (show other bugs)
: unspecified
: All All
: P1 normal
: Firefox 16
Assigned To: Myk Melez [:myk] [@mykmelez]
: Jason Smith [:jsmith]
Mentors:
: 758887 763429 (view as bug list)
Depends on:
Blocks: 751310 751413 754396 755077
  Show dependency treegraph
 
Reported: 2012-05-07 13:42 PDT by Myk Melez [:myk] [@mykmelez]
Modified: 2016-03-21 12:39 PDT (History)
21 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch v1: implements plan (6.63 KB, patch)
2012-05-28 22:48 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Review
patch v2: implements gavin's suggestions (6.50 KB, patch)
2012-05-29 16:40 PDT, Myk Melez [:myk] [@mykmelez]
felipc: review+
Details | Diff | Review
patch v3: addresses review nits (6.13 KB, patch)
2012-06-11 10:45 PDT, Myk Melez [:myk] [@mykmelez]
myk: review+
myk: checkin+
Details | Diff | Review

Description Myk Melez [:myk] [@mykmelez] 2012-05-07 13:42:27 PDT
The runtime currently employs a heuristic to determine when to load a URL in the user's default browser instead of the runtime.  The heuristic is that we load all on-origin URLs in the runtime and all off-origin URLs in the browser, except for URLs to a hard-coded set of off-origin identity providers, which load in the runtime (subject to the conditions of bug 741954).

This heuristic is problematic because apps sometimes want to load on-origin URLs in the browser (f.e. a support website for the app) and other off-origin URLs in the runtime (f.e. an identity provider that isn't on our hard-coded list).  We should replace it with an explicit mechanism by which an app can identify the target of a URL load.
Comment 1 Myk Melez [:myk] [@mykmelez] 2012-05-07 13:49:03 PDT
Ian Bicking told me that Ben Francis has suggested using anchor targets to identify the load target of links.

For example, we could define a keyword, _mozBrowser (underscore to indicate that the keyword has a special meaning; "moz" to identify the keyword as a non-standard Mozilla extension), to which the `target` attribute of <a> tags is set to identify links that should open in the browser.

When an app runs in the runtime, links whose target is not _mozBrowser would load in the runtime, regardless of their origin (subject to the conditions of bug 741954).  Links whose target *is* _mozBrowser, on the other hand, would load in the user's default browser.

When an app runs in a browser, on the other hand, links whose target is not _mozBrowser would load in the browser's conventional location for the link.  Links whose target is _mozBrowser would load in a new tab/window, as _mozBrowser would be treated as the name of a browsing context.
Comment 2 Myk Melez [:myk] [@mykmelez] 2012-05-07 14:52:07 PDT
Ian Bicking told me that Ben Francis has suggested using anchor targets to identify the load target of links.

For example, we could define a keyword, _mozBrowser (underscore to indicate that it's a keyword; "moz" to identify the keyword as a non-standard Mozilla extension), to which the `target` attribute of <a> tags is set to identify links that should open in the browser.

When an app runs in the runtime, links whose target is not _mozBrowser would load in the runtime, regardless of their origin (subject to the conditions of bug 741954).  Links whose target *is* _mozBrowser, on the other hand, would load in the user's default browser.

When an app runs in a browser, on the other hand, links whose target is not _mozBrowser would load in the browser's conventional location for the link.  Links whose target is _mozBrowser would load in a new tab/window, as _mozBrowser would be treated as the *name* of a browsing context rather than a keyword.
Comment 3 Myk Melez [:myk] [@mykmelez] 2012-05-07 14:53:49 PDT
Without a fix for this bug, apps will have trouble under a variety of circumstances when loading URLs, and the workaround we've currently employed for a hardcoded handful of identity providers does not feel sufficiently robust or comprehensive to be shippable, so we should fix this for k9o.
Comment 4 Ian Bicking (:ianb) 2012-05-07 15:03:31 PDT
Would (re)using _blank work for this case?
Comment 5 Myk Melez [:myk] [@mykmelez] 2012-05-07 15:11:17 PDT
Reusing _blank would give apps a mechanism for loading URLs in the browser, but it would prevent apps from using it to load URLs in new windows in the runtime.  That might be ok, since they can still use window.open() to do that; or it might be onerous, because they'll have to use window.open() to do that; not sure.
Comment 6 Ian Bicking (:ianb) 2012-05-07 15:21:55 PDT
Thinking about when web apps use window.open(), and when they use <a target=_blank>, it seems to me to line up with the notion of opening in a sub-window (inside the app) and in an external browser.  The primary concern though would be cases when you might want something like target=_blank in an app.  (Though without tabs an indefinite number of windows becomes unwieldy, and if you have a well defined number of windows you could use target=some_name).
Comment 7 Jason Smith [:jsmith] 2012-05-07 21:26:44 PDT
Would this remove the need for adding an allowed_origins to the manifest for web apps, as this gives the app full control of where resources are loaded within the app context?
Comment 8 Ben Francis [:benfrancis] 2012-05-08 13:31:52 PDT
I don't think I suggested a "_mozBrowser" target and I'm not sure if that's a good idea.

Using target=_blank inside an app when you want to open a hyperlink in the default browser app seems like it could work.

window.open could be more problematic. In B2G apps will only be allowed to have one window, window.open simply won't work. Both window.open and target="_blank" will be supported in the browser app though, and will open a new browser tab.
Comment 9 Myk Melez [:myk] [@mykmelez] 2012-05-08 13:32:11 PDT
(In reply to Ian Bicking (:ianb) from comment #6)
> Thinking about when web apps use window.open(), and when they use <a
> target=_blank>, it seems to me to line up with the notion of opening in a
> sub-window (inside the app) and in an external browser.  The primary concern
> though would be cases when you might want something like target=_blank in an
> app.  (Though without tabs an indefinite number of windows becomes unwieldy,
> and if you have a well defined number of windows you could use
> target=some_name).

Named targets are good for the use case of opening or focusing a singleton window, but they don't address the use case of opening a(nother) new window, which is what target=_blank is for.

And using it in the webapp runtime might be unwieldy, but target=_blank is nevertheless part of the web platform and thus not to be overloaded lightly (and could be implemented via tabs in the future if warranted).

Finally, while overloading target=_blank is a better heuristic than the one we have, it's still a heuristic, and it would complicate the appification of websites that use that feature today.

This discussion would benefit from input from DOM folks.  cc:ing a couple.

Johnny, Jonas: what do y'all think?
Comment 10 Myk Melez [:myk] [@mykmelez] 2012-05-08 13:34:06 PDT
(In reply to Jason Smith [:jsmith] from comment #7)
> Would this remove the need for adding an allowed_origins to the manifest for
> web apps, as this gives the app full control of where resources are loaded
> within the app context?

Yes, fixing this bug addresses the same issue that prompted consideration (in a separate bug) of an "allowed origins" field in the manifest.
Comment 11 Ed Lee :Mardak 2012-05-08 13:40:06 PDT
Myk meant to add jst and sicking in comment 9.
Comment 12 Myk Melez [:myk] [@mykmelez] 2012-05-08 13:49:01 PDT
(In reply to Ben Francis from comment #8)
> I don't think I suggested a "_mozBrowser" target and I'm not sure if that's
> a good idea.

Sorry that it sounded like I was attributing that to you; it isn't the case.  I heard that you suggested using anchor targets, but I was the one who came up with the idea of adding a _mozBrowser link target as an example of how we might use anchor targets to solve the problem.  I'm not sure it's a good idea either, but at the moment it isn't clear what the better one would be.

Another option is to mark such links via an additional anchor attribute to avoid the side-effects of reusing `target`.  For example, we could add a mozDisposition attribute (where "moz" is again a way of identifying this symbol as a non-standard Mozilla extension, and "disposition" is intended to be analogous to the "Content-Disposition" header, which can be used to direct browsers to download a resource instead of displaying it) that, when present and set to "browser", would cause the link to load in the browser.


> window.open could be more problematic. In B2G apps will only be allowed to
> have one window, window.open simply won't work. Both window.open and
> target="_blank" will be supported in the browser app though, and will open a
> new browser tab.

window.open currently works in the desktop runtime, and it seems like something we want to continue to support in that runtime, given that it's a key feature of desktop OSes.  Which makes this one of the (hopefully relatively limited) cases in which the web platform differs between runtimes (and the browser).


(In reply to Edward Lee :Mardak from comment #11)
> Myk meant to add jst and sicking in comment 9.

Yes, thanks Ed!  Those additions got lost in a mid-air alert that I blithely told to "submit only my comment."
Comment 13 Ed Lee :Mardak 2012-05-08 14:08:30 PDT
(In reply to Myk Melez [:myk] [@mykmelez] from comment #12)
> (In reply to Ben Francis from comment #8)
> > window.open could be more problematic. In B2G apps will only be allowed to
> > have one window, window.open simply won't work.
> window.open currently works in the desktop runtime, and it seems like
> something we want to continue to support in that runtime, given that it's a
> key feature of desktop OSes.
This discrepancy in runtimes also affects the Android implementation. The Fennec runtime would need to somehow support multiple views/panels/tabs because the OS doesn't provide native handling of "multiple windows."

Do we have actual uses of requiring multiple windows in an app?

Right now we're running into this problem to try to be graceful in appification of websites that expected a tabbed/multi-window user agent.

We can avoid Persona login issues with natively supporting navigator.id, but even without that, an app-targeted reworking of a website could potentially use iframes for login (assuming allowing cross domain communication and ignoring the non-chrome-indicating security issues).
Comment 14 Jonas Sicking (:sicking) 2012-05-08 22:21:16 PDT
This is a complex enough issue that I don't feel like I have any great solutions.

My gut feeling is that declaring what should be loaded in-app vs. in-browser is best declared in the manifest. It's a bit of a pain in the ass to have to add a target="..." attribute everywhere where you want links to open in an external window.

Possibly we could default to having in-origin URLs always load in-app and out-of-origin load in-browser, but then allow the manifest to overload in both directions.
Comment 15 Jason Smith [:jsmith] 2012-05-08 23:34:21 PDT
(In reply to Jonas Sicking (:sicking) from comment #14)
> This is a complex enough issue that I don't feel like I have any great
> solutions.
> 
> My gut feeling is that declaring what should be loaded in-app vs. in-browser
> is best declared in the manifest. It's a bit of a pain in the ass to have to
> add a target="..." attribute everywhere where you want links to open in an
> external window.

We used to have a bug tracking this, but it was marked as a won't fix. See bug 749415.

> 
> Possibly we could default to having in-origin URLs always load in-app and
> out-of-origin load in-browser, but then allow the manifest to overload in
> both directions.

Right, that's pretty much what bug 749415 said.
Comment 16 Jason Smith [:jsmith] 2012-05-08 23:37:07 PDT
Based on watching the discussion on this bug plus bug 749415, I do wonder if it be worth to have a brainstorming session to solve the off-origin resources problem. Feels like we are in a circular discussion right now. Thoughts?
Comment 17 Ben Francis [:benfrancis] 2012-05-09 10:04:56 PDT
(In reply to Jonas Sicking (:sicking) from comment #14)
> Possibly we could default to having in-origin URLs always load in-app and
> out-of-origin load in-browser, but then allow the manifest to overload in
> both directions.

That makes a lot of sense. It isn't clear what target=_blank links would do in single window mobile apps though.
Comment 18 Ben Francis [:benfrancis] 2012-05-09 10:18:25 PDT
cjones suggests that in B2G both target=_blank and window.open used inside an app could fire a Web Intent to open the URL in the default browser app. The exception to this is the use case of third party authentication for which we may need a new mechanism/API.

So by default in B2G same-origin links should open in the same window, cross-origin, target=_blank and window.open should fire an intent. Then you could somehow specify exceptions to this in the manifest. You may want the default behavior be different on desktop.
Comment 19 Ed Lee :Mardak 2012-05-09 10:43:59 PDT
(In reply to Ben Francis from comment #18)
> So by default in B2G same-origin links should open in the same window,
> cross-origin, target=_blank and window.open should fire an intent.
I would assume we would want the app user experience to be the same across desktop runtime, mobile, and b2g.

If the main use case is to popup an authentication box, perhaps we should just focus on that. So on all platforms, we could do something like allowing one popup that acts like a modal dialog that sits over the app content. It could be implemented as a xul:browser stacked over the main app content's xul:browser with some styling and a close button.
Comment 20 Jason Smith [:jsmith] 2012-05-09 11:14:17 PDT
(In reply to Edward Lee :Mardak from comment #19)
> (In reply to Ben Francis from comment #18)
> > So by default in B2G same-origin links should open in the same window,
> > cross-origin, target=_blank and window.open should fire an intent.
> I would assume we would want the app user experience to be the same across
> desktop runtime, mobile, and b2g.

Right, this follows in line with the vision of apps. Build once, run anywhere. We can't take risks associated with violating this vision where one platform does something different unless there is a really strong reason why.

> 
> If the main use case is to popup an authentication box, perhaps we should
> just focus on that. So on all platforms, we could do something like allowing
> one popup that acts like a modal dialog that sits over the app content. It
> could be implemented as a xul:browser stacked over the main app content's
> xul:browser with some styling and a close button.

Note - Another use case we've seen in facebook like buttons and google +1 buttons, so there could be other things to look out for.
Comment 21 Ed Lee :Mardak 2012-05-09 11:20:30 PDT
(In reply to Jason Smith [:jsmith] from comment #20)
> Note - Another use case we've seen in facebook like buttons and google +1
> buttons, so there could be other things to look out for.
I'm not sure how that is related unless clicking a like button redirects the whole page to Facebook or opens a popup window.. ?
Comment 22 Justin Lebar (not reading bugmail) 2012-05-09 11:27:48 PDT
(In reply to Edward Lee :Mardak from comment #21)
> (In reply to Jason Smith [:jsmith] from comment #20)
> > Note - Another use case we've seen in facebook like buttons and google +1
> > buttons, so there could be other things to look out for.
> I'm not sure how that is related unless clicking a like button redirects the
> whole page to Facebook or opens a popup window.. ?

And if it does, the redirect to Facebook or the popup window should of course happen in the browser.

The main problem I see with this unified vision is: Are we ready to commit to saying that desktop apps never have more than one window?  B2G wants one-app-per-window.  If desktop wants to do something different, then that's of course a significant difference between desktop and mobile.
Comment 23 Myk Melez [:myk] [@mykmelez] 2012-05-09 11:29:59 PDT
(In reply to Edward Lee :Mardak from comment #19)
> I would assume we would want the app user experience to be the same across
> desktop runtime, mobile, and b2g.

No, we want it to be consistent across devices/platforms in some cases and consistent with device/platform conventions/capabilities in others.


(In reply to Jason Smith [:jsmith] from comment #20)
> Right, this follows in line with the vision of apps. Build once, run
> anywhere.

That vision does not mean we should only implement the lowest-common-denominator development platform and user experience.  We must make these decisions on a case-by-case basis based on user/developer expectations and benefits.
Comment 24 Jason Smith [:jsmith] 2012-05-09 11:59:14 PDT
(In reply to Edward Lee :Mardak from comment #21)
> (In reply to Jason Smith [:jsmith] from comment #20)
> > Note - Another use case we've seen in facebook like buttons and google +1
> > buttons, so there could be other things to look out for.
> I'm not sure how that is related unless clicking a like button redirects the
> whole page to Facebook or opens a popup window.. ?

It does, although resolving the off-origin pop-up problem would hit that problem too. See bug 751054.

(In reply to Justin Lebar [:jlebar] from comment #22)
> (In reply to Edward Lee :Mardak from comment #21)
> > (In reply to Jason Smith [:jsmith] from comment #20)
> > > Note - Another use case we've seen in facebook like buttons and google +1
> > > buttons, so there could be other things to look out for.
> > I'm not sure how that is related unless clicking a like button redirects the
> > whole page to Facebook or opens a popup window.. ?
> 
> And if it does, the redirect to Facebook or the popup window should of
> course happen in the browser.

Not sure if I agree. Off-origin social integration is likely within an app as per bug 751064, which probably should follow the same use case as we do with account management off-origin (e.g. facebook login, google accounts login).

> 
> The main problem I see with this unified vision is: Are we ready to commit
> to saying that desktop apps never have more than one window?  B2G wants
> one-app-per-window.  If desktop wants to do something different, then that's
> of course a significant difference between desktop and mobile.

Right. One app per window I don't think sounds feasible based on initial testing we've seen the desktop runtime (see the web apps --> desktop runtime bugs for examples) against the reality of how apps are developed, as for example, off-origin account management is a likely use case with top tier applications.

(In reply to Myk Melez [:myk] [@mykmelez] from comment #23)
> (In reply to Edward Lee :Mardak from comment #19)
> > I would assume we would want the app user experience to be the same across
> > desktop runtime, mobile, and b2g.
> 
> No, we want it to be consistent across devices/platforms in some cases and
> consistent with device/platform conventions/capabilities in others.
> 

Right, understood. Both factors need to be taken into account. The problem in this bug though I think the underlying problem affects all platforms (desktop firefox, fennec native, and b2g), as this relates to how to handle off-origin content within an application. Top apps testing during MWC prep, mobile web compat analysis, and desktop webRT testing all of have shown this problem to be a reality across all platform types, so we need a mitigation strategy that works both for each platform that makes sense within the k9o vision.

> 
> (In reply to Jason Smith [:jsmith] from comment #20)
> > Right, this follows in line with the vision of apps. Build once, run
> > anywhere.
> 
> That vision does not mean we should only implement the
> lowest-common-denominator development platform and user experience.  We must
> make these decisions on a case-by-case basis based on user/developer
> expectations and benefits.

Right. My point I'm trying to make here is paying attention to the platform needs, the vision needs, and how it all integrates together. With this particular problem in this bug, there needs to be a resolution across all platforms that meets each of those needs stated before.

I think at this point, it's time for a cross-functional discussion on this problem, as this bug discussion is in a circular argument. I could set a discussion up if people are interested with going forward with this. Should we go forward with a discussion on this?
Comment 25 Jason Smith [:jsmith] 2012-05-11 13:08:11 PDT
*** Bug 754396 has been marked as a duplicate of this bug. ***
Comment 26 Myk Melez [:myk] [@mykmelez] 2012-05-16 15:51:40 PDT
Jonas and I talked through the various issues yesterday and concluded that reusing the "_blank" keyword, as Ian suggests in comment 4, and as B2G may be doing per comment 18, is the most reasonable way to identify link clicks that should open in the browser.

An window can't access a _blank window it opens via a link click (although the _blank window can access its opener), and _blank windows seem to have limited utility to apps, which we expect to want to open specific sets of named windows.

Exceptions exist, namely apps that provide links to third-party content, like Twitter and Facebook, which use _blank to identify those links.  But that is a compatible use, as the native versions of those apps load those links in the user's default browser.

And in truly exceptional cases, there are simple workarounds.  Apps can add _blank to links that should open in a browser and remove them from those that shouldn't.  And if an app really needs the _blank behavior (opening an arbitrary new window) within the app, it can give each link target a unique name.

We also concluded that the desktop runtime should support multiple windows on desktop platforms, not only to support identity providers (including Mozilla's own) but because it is a longstanding key feature of those platforms.  In this case, consistency with the desktop platforms is more valuable than consistency across platforms, which vary considerably in this regard.

And we separated the security concern about limiting the content an app can load, since the initial version of the desktop runtime only supports untrusted apps, as defined by the Apps Security Model <https://wiki.mozilla.org/Apps/Security>, and untrusted apps may load content from any origin (subject to Same Origin Policy restrictions), just like normal web content, to make it easy for web developers to port their sites to apps.

Thus the plan for this bug is to remove the heuristic that determines where to load a link based on a hardcoded whitelist, such that navigation happens in the app by default; and then make links whose target is the keyword "_blank" open in the browser.

(We leave open the question of where to open URLs in window.open calls whose strWindowName parameter is "_blank".  The behavior of such calls is similar to _blank links, but the caller gets a reference to the opened window, so app developers might use them in different ways.  We'll keep an eye on this issue.)

(Jonas' concern about a cross-origin frame/window navigating its top/opener is also a valid issue, but we concluded that it can be addressed separately, so I will file another bug about it.)
Comment 27 Jason Smith [:jsmith] 2012-05-16 16:02:02 PDT
Especially for this extent of the proposal provided in comment 26, I'd highly suggest build a prototype first (e.g. try build) to see if this concept adapts well to existing applications, specifically top apps. With the prototype, I can take a look at whether the proposal will work or not.
Comment 28 Andrew Williamson [:eviljeff] 2012-05-17 04:25:38 PDT
I'm not convinced that loading all links in-app by default is the best solution for the user.

The majority of apps I've reviewed have had a least one external link somewhere within the app (some, many) and my concern is once a user navigates to that link and leaves the confines of the designed app content they're stuck in the wider internet.   

All apps are untrusted currently so there aren't any particular extra security issues but the internet generally isn't suited to a UI without any navigation aids - i.e. there is no Back button; there is no url bar to indicate where you are; there is no ssl security indicator; there is no home button to get you back to the app.  
Users' only option will be to close down the app and relaunch it, which is imo a poor experience.

Can someone (Myk?) expand on why this approach was settled upon?

I'd favour a solution that switches the assumption towards off-origin links being launched in a browser and lets developers specify which links/sites they explicitly want launched in-app.  So, either an additional whitelist in the manifest or a specific target in the links they want loaded in-app ("_inapp", "_app" ?)
Comment 29 Justin Lebar (not reading bugmail) 2012-05-17 07:26:48 PDT
This is important, but I don't quite understand:

> We also concluded that the desktop runtime should support multiple windows on desktop platforms, not 
> only to support identity providers (including Mozilla's own) but because it is a longstanding key 
> feature of those platforms.  In this case, consistency with the desktop platforms is more valuable 
> than consistency across platforms, which vary considerably in this regard.

So _blank on desktop means "open in new app window" whereas _blank on mobile means "open in browser app"?

If so, I have some questions:

 1) If this is our identity provider solution, how does one log into BrowserID/FB Connect on mobile?
 2) How does an app developer know ahead of time whether _blank is going to open in the browser or a new app window?
 3) How does the user authenticate that they're actually logging into FB connect and not being phished?  Isn't one of the key purposes of a federated login system that I don't have to trust third parties not to steal my passwords?

From my perspective, a desktop app opening multiple windows for purposes other than authentication is an edge case we don't need to optimize for.  If we want to support it, that's great, but making it easy isn't important.  In contrast, I understood that figuring out authentication on all platforms was important to us.

Here's a counter-proposal:

 a) _blank always opens in the browser.
 b) We introduce a new _auth target which opens a modal window with chrome showing the location of the page.  We do our best to make this window difficult to phish (this is hard, but we should at least try).
 c) We have a whitelist of identity providers whose _blank loads are converted to _auth, so the big identity providers will Just Work.
 d) We introduce a new _blankapp (or whatever) target which opens a window in the app itself.  Pages will be able to tell whether a call to window.open(..., "_blankapp") succeeded by looking at the return value, so we don't have to promise that it will always work.
Comment 30 Ian Bicking (:ianb) 2012-05-17 09:02:20 PDT
Would it be feasible to restore navigation (back/forward/url bar) for off-origin pages?  This would avoid some developer confusion if they accidentally change origin (e.g., redirect to from http://someapp.com to http://www.someapp.com), and also give the user a way to get out of that context.

Re: comment 29, my assumption for multiple windows is that we'd expect app developers to do something like <a target="user-overview" href="/user-overview">See users</a> to launch multiple windows (or perhaps more likely window.open("/user-overview", "user-overview")).  An app can always generate random window names if it wants unlimited new in-app windows.
Comment 31 Justin Lebar (not reading bugmail) 2012-05-17 09:14:42 PDT
> Would it be feasible to restore navigation (back/forward/url bar) for off-origin pages? 

Feasible, certainly.  But desirable?  It would be pretty confusing to press "back" and then have all the "browser chrome" disappear.  Not to mention the fact that seeing browser chrome completely kills the "illusion" of being in an app.

This feels like a hack to me.

> Re: comment 29, my assumption for multiple windows is that we'd expect app developers to do 
> something like <a target="user-overview" href="/user-overview">See users</a> to launch multiple 
> windows (or perhaps more likely window.open("/user-overview", "user-overview")).  An app can always 
> generate random window names if it wants unlimited new in-app windows.

That works for window.open.  But how does the app know ahead of time what's going to happen when the user clicks on a link with a random target?  Presumably on mobile we'd block it because it's not _blank, and on desktop we'd open a new window?

We could make it so that you can't open new app windows via <a target>.  Like I said, this is an edge case, and we have much more important things to optimize for.
Comment 32 Andrew Williamson [:eviljeff] 2012-05-17 09:23:47 PDT
(In reply to Justin Lebar [:jlebar] from comment #31)
> > Would it be feasible to restore navigation (back/forward/url bar) for off-origin pages? 
> 
> Feasible, certainly.  But desirable?  It would be pretty confusing to press
> "back" and then have all the "browser chrome" disappear.  Not to mention the
> fact that seeing browser chrome completely kills the "illusion" of being in
> an app.
> 
> This feels like a hack to me.

I agree.  If the user has gone to a page where they need navigation then its not an app anymore and we're much better pushing it back to the browser where they'll have the full navigation options (and any other customisation they're used to)
Comment 33 Ian Bicking (:ianb) 2012-05-17 09:39:11 PDT
I think we have two use cases for keeping off-origin pages in the app, one strong and clear, and another somewhat vague.  

The strong use case is auth (including OAuth authorization, of which an app might support several for good reasons).  In these cases the user navigates away from the app to the auth flow, and the auth flow communicates something back to the app when the flow is finished (either by redirecting back to the app or by using window.opener).  I don't think it's important in this case that the auth flow look like an app – it's not an app, it's a web page, and you will probably have to enter your credentials for the other service (i.e., if the Foobar app uses Facebook, and starts an auth flow, you enter your Facebook username and password, not your Foobar username and password).  It's important that the auth flow can communicate back to the app, and we have no way for external browser windows to do that.

The second use case is that an app actually wants to display an off-origin page, and it doesn't want to frame that in an iframe.  I can't come up with very good reasons for this case.  A help system hosted in an off-origin CMS?  Why not keep it in an iframe?  It probably needs navigation anyway (which the iframe can't really provide... but that's a completely different problem).

And I thought of a third case: downloading off-origin content (that might be off-origin for good operational reasons, or for security).  That could open a browser window that immediately becomes a download (something that is common now with many PDF links), but it would be better if it actually just downloaded the file without the browser window intermediary.  This use case could be handled by this: http://www.whatwg.org/specs/web-apps/current-work/multipage/links.html#attr-hyperlink-download
Comment 34 Myk Melez [:myk] [@mykmelez] 2012-05-17 11:01:22 PDT
(In reply to Andrew Williamson [:eviljeff] from comment #28)
> The majority of apps I've reviewed have had a least one external link
> somewhere within the app (some, many) and my concern is once a user
> navigates to that link and leaves the confines of the designed app content
> they're stuck in the wider internet.

If those links specify a _blank target, they'll open in the browser, so the user won't leave the confines of the designed app content.

And if they don't, the fix is simple: add target="_blank" to those links.


> All apps are untrusted currently so there aren't any particular extra
> security issues but the internet generally isn't suited to a UI without any
> navigation aids - i.e. there is no Back button; there is no url bar to
> indicate where you are; there is no ssl security indicator; there is no home
> button to get you back to the app.  

Right.  However, we're not designing a runtime for the internet in general, since we already have one of those (the browser).  We're designing one for apps, which are purpose-built enclosed environments that provide their own navigation.

We do want to make it as easy as possible to migrate a website to an app, and that goal motivates the decisions being made here.  But developers may nevertheless have to make some changes to their websites (if they haven't already done so) to accommodate a runtime that doesn't provide browser-like navigational affordances.


> Users' only option will be to close down the app and relaunch it, which is
> imo a poor experience.

Indeed.  But websites designed as apps (f.e. Facebook, Twitter, Gmail, Google News) are already using _blank to open external links in new windows/tabs, and developers can add that target easily enough.


> Can someone (Myk?) expand on why this approach was settled upon?

We're balancing a number of concerns, especially the ease of developer transition from the web-platform-in-browser to the web-platform-in-runtime, existing conventions for app-like websites, and the user experience of apps (particularly their consistency with native apps and across platforms).

Jonas and I think this proposal makes the best set of trade-offs between those concerns, because it enables apps to use third-party identity providers without changes, reuses existing conventions for browsing to external links, is consistent with the appearance and behavior of native apps, and may also be relatively consistent with behavior on B2G (and Android, once we implement a runtime there).
Comment 35 Myk Melez [:myk] [@mykmelez] 2012-05-17 11:14:54 PDT
(In reply to Justin Lebar [:jlebar] from comment #29)
> So _blank on desktop means "open in new app window" whereas _blank on mobile
> means "open in browser app"?

No, _blank means the same on both platforms: open in browser app.  But a named target on desktop opens in a new app window.


> If so, I have some questions:
> 
>  1) If this is our identity provider solution, how does one log into
> BrowserID/FB Connect on mobile?

This is not our identity provider solution, although one of the goals of this bug is to ensure existing integration with identity providers works as expected.  And this bug is specific to desktop, although our decisions are informed by those being made for mobile platforms.


>  2) How does an app developer know ahead of time whether _blank is going to
> open in the browser or a new app window?

On desktop, _blank will always open in the browser.


>  3) How does the user authenticate that they're actually logging into FB
> connect and not being phished?  Isn't one of the key purposes of a federated
> login system that I don't have to trust third parties not to steal my
> passwords?

We discussed this during the runtime's security review and agreed to initially put the domain of the loaded page into the window's titlebar whenever an app navigates off-origin (pending further investigation of better affordances for providing this information to users).


> Here's a counter-proposal:
> 
>  a) _blank always opens in the browser.

That is the plan.


>  b) We introduce a new _auth target which opens a modal window with chrome
> showing the location of the page.  We do our best to make this window
> difficult to phish (this is hard, but we should at least try).
>  c) We have a whitelist of identity providers whose _blank loads are
> converted to _auth, so the big identity providers will Just Work.
>  d) We introduce a new _blankapp (or whatever) target which opens a window
> in the app itself.  Pages will be able to tell whether a call to
> window.open(..., "_blankapp") succeeded by looking at the return value, so
> we don't have to promise that it will always work.

These steps don't seem necessary, as the _auth and _blankapp targets are equivalent to a named target, which we already support (and expect to be preferable, as native apps typically open specific windows rather than arbitrary ones).  And we already plan to use titlebar chrome to show the location of the page whenever a window navigates off-origin.

Also note that auth is not necessarily modal, and requiring it to be adds complexity to the model.  And it doesn't always take place in a new window; some providers use an interstitial.
Comment 36 Myk Melez [:myk] [@mykmelez] 2012-05-17 11:26:26 PDT
(In reply to Ian Bicking (:ianb) from comment #33)
> The second use case is that an app actually wants to display an off-origin
> page, and it doesn't want to frame that in an iframe.  I can't come up with
> very good reasons for this case.

One use case is apps and identity providers hosted on multiple origins.  I've seen apps in the wild whose domain references are inconsistently prefixed with "www.".  And identity providers that redirect from domain to domain.  Also, Jonas suggested that apps using content delivery networks don't always so do transparently and leak multiple domain names into links.

We will be able to do much better with trusted and certified apps, whose chrome resources will be specified and restricted.  But the current plan seems like the best compromise between conflicting goals for the untrusted apps we will be initially supporting.
Comment 37 Justin Lebar (not reading bugmail) 2012-05-17 11:30:57 PDT
> This is not our identity provider solution, although one of the goals of this bug is to ensure 
> existing integration with identity providers works as expected.

I think maybe we're not in sync about what I meant by "this is our identity provider solution".  What I mean is, the proposal here needs to make sure that we can use third-party identity providers without changes, like you said in comment 34.

I don't understand how this works, as proposed.

_blank always opens in the browser.  So I'm running my app, and it uses FB connect.  FB connect calls window.open(_blank).  Now we load the browser.  Now what?

Setting aside what sounds like an awful user experience, where we load a full instance of Firefox just so you can log in to an app, is your idea that the browser is capable of communicating the auth credentials back to the app?  That has a lot of implications, but foremost, it means that _blank doesn't open in "the browser" -- it has to open in *Firefox*.  That seems heavy-handed and not in our users' interests.

It seems to me that auth is an entirely different use-case, with entirely different requirements, from "open in the browser".  By using _blank for both of them, we conflate these two use-cases.

But maybe I'm misunderstanding you.
Comment 38 Jonas Sicking (:sicking) 2012-05-17 11:48:19 PDT
Justin: It is absolutely *not* the idea that something that opens in "the browser" should communicate back to the app runtime. Nor should the app runtime communicate with something that has opened in "the browser".

The idea is that facebook-connect would use window.open(somename) in order to display the window which they can then communicate with. Our hope was that websites usually use <a href=... target=_blank> for when they want to open thing "not in my tab" which would translate to "not in my app". I.e. for twitter/facebook/gmail that want to open user-provided links. And that they would use window.open(somename) and <a href=... target=somename> when they want to open a window that they have control over and that they want to communicate with. I.e. for login or general multi-window UIs.

We actually left it a bit undefined where window.open(_blank) would open, mostly because we didn't think about it.

If it's the case that many websites today open things using window.open(_blank) and expect to communicate with it, then we should treat that as opening in the app rather than in "the browser".

I definitely think that window.open(_blank) is the most complex case here. It seems like the one which is most commonly used today for *both* for opening external links, and for opening multi-window UIs.
Comment 39 Justin Lebar (not reading bugmail) 2012-05-17 12:11:46 PDT
Ah, I understand.  I didn't realize we expected identity providers to be using named windows rather than "_blank".  I just looked at FB connect, and it does

  window.open("https://facebook.com/dialog/oauth?lots_of_stuff", <hash>)

so that would work with the current proposal.

However, BrowserID apparently does

  window.open("https://browserid.org/sign_in")

with no window name.  So it sounds like we'd need to conduct a survey of Important Auth Providers in order to discover whether this proposal would work in general.
Comment 40 Ben Francis [:benfrancis] 2012-05-18 06:01:06 PDT
Data on this would certainly help.

It sounds like you're now talking about apps having multiple windows again (for third party authentication use cases), which I thought wouldn't be possible in B2G? Maybe I'm missing something?
Comment 41 Justin Lebar (not reading bugmail) 2012-05-18 07:51:43 PDT
Yes, like Ben says, this solution doesn't help us much on mobile.  We'll still need to have some way to explicitly tell that window.open is for auth, because we're likely to block all other window.open calls.

We can do that independently, of course.  But I don't see how that's preferable to having the desktop team on-board.
Comment 42 Myk Melez [:myk] [@mykmelez] 2012-05-18 10:26:56 PDT
(In reply to Justin Lebar [:jlebar] from comment #41)
> Yes, like Ben says, this solution doesn't help us much on mobile.  We'll
> still need to have some way to explicitly tell that window.open is for auth,
> because we're likely to block all other window.open calls.
> 
> We can do that independently, of course.  But I don't see how that's
> preferable to having the desktop team on-board.

Indeed, it would be better for the desktop team to be involved in the B2G decision-making process so we can be informed about what B2G plans to do and provide input into those plans as appropriate (just as B2G folks are providing input into desktop's plans here).
Comment 43 Justin Lebar (not reading bugmail) 2012-05-18 10:37:51 PDT
Please consider yourself involved!  The only B2G-specific decision here is that we don't want multiple windows per app as a general use-case (i.e., we'll support it for auth, but that's it).

Is that a decision you'd like to revisit?
Comment 44 Myk Melez [:myk] [@mykmelez] 2012-05-18 10:46:39 PDT
(In reply to Justin Lebar [:jlebar] from comment #43)
> Please consider yourself involved!  The only B2G-specific decision here is
> that we don't want multiple windows per app as a general use-case (i.e.,
> we'll support it for auth, but that's it).
> 
> Is that a decision you'd like to revisit?

I don't know enough about B2G to have an informed opinion yet.  I could see it using windows similarly to the way Android uses activities <http://developer.android.com/guide/topics/fundamentals/activities.html>, but perhaps y'all have other plans for the user interaction model, and windows don't map well onto it.
Comment 45 Justin Lebar (not reading bugmail) 2012-05-18 10:56:56 PDT
We're using Web Intents (née Web Activities) like Android uses Activities.

An application on Android or iOS has only one window from the OS's perspective -- the OS does not implement an intra-app window manager.  For example, you cannot use an OS mechanism to close one window of an app; you can only close things at the app granularity.  The intent is to do the same on B2G.
Comment 46 Ed Lee :Mardak 2012-05-18 11:04:33 PDT
(In reply to Justin Lebar [:jlebar] from comment #41)
> Yes, like Ben says, this solution doesn't help us much on mobile.  We'll
> still need to have some way to explicitly tell that window.open is for auth,
> because we're likely to block all other window.open calls.
jlebar, what happens on b2g when an app has an anchor targeting _blank?

And related, do cookies get shared from the browser app and other apps? Because if they do, an app could accidentally be already authed if a user signed in from the browser app or even any other app, right?
Comment 47 Justin Lebar (not reading bugmail) 2012-05-18 11:06:57 PDT
> jlebar, what happens on b2g when an app has an anchor targeting _blank?

I think we're going to open it in the browser (using an intent).

> And related, do cookies get shared from the browser app and other apps?

Likely not, otherwise bad things happen, like you say.
Comment 48 Andrew Williamson [:eviljeff] 2012-05-18 11:22:11 PDT
(In reply to Myk Melez [:myk] [@mykmelez] from comment #34)
> Right.  However, we're not designing a runtime for the internet in general,
> since we already have one of those (the browser).  We're designing one for
> apps, which are purpose-built enclosed environments that provide their own
> navigation.
> 
> We do want to make it as easy as possible to migrate a website to an app,
> and that goal motivates the decisions being made here.  But developers may
> nevertheless have to make some changes to their websites (if they haven't
> already done so) to accommodate a runtime that doesn't provide browser-like
> navigational affordances.

Ideally all apps would use target="_blank" to load links to other sites and the developers of those apps would be careful to only load off-origin links in-app when they were part of the app.  Unfortunately most apps aren't perfectly written (from the ones I've reviewed on Marketplace so far), and a significant number of them are based on (or directly implemented with) existing app-like webpages that won't have been that carefully written.  

When an off-origin link is loaded in the app and a user gets stuck on a normal website with no way of getting back its a really bad user experience.  Imo we need to make it difficult for developers to do write html that does potentially bad things like that.  Forcing app developers to effectively 'opt-in' to an off-origin link being loaded in-app will help do that.

(None of this affects the rest of the issues in the bug re: authentication providers and b2g that I can see - it just changes the default assumption where there is no target specified)
Comment 49 Jason Smith [:jsmith] 2012-05-26 16:00:34 PDT
*** Bug 758887 has been marked as a duplicate of this bug. ***
Comment 50 Myk Melez [:myk] [@mykmelez] 2012-05-28 22:48:52 PDT
Created attachment 627856 [details] [diff] [review]
patch v1: implements plan

This patch implements the plan that Jonas and I worked out.  In particular:

* <a target="_blank"> loads in the browser
* <a target="something else"> loads in the runtime
* window.open(url, "anything") loads in the runtime

Note: the behavior of window.open(url, "_blank") is thus to load the URL in a new blank window in the runtime.  We'll want to do more research to figure out if that's the best behavior and then modify it as appropriate, but that's another bug.

-> Felipe for review of the code, which is all in the Firefox module.

-> Jonas for feedback on this patch's fidelity to the plan and its pure browser chrome implementation strategy.  I looked around a bit for some other way of intercepting these clicks, but I didn't see one.  In particular, neither nsIContentPolicy nor nsIURIContentListener seemed usable.
Comment 51 Ben Francis [:benfrancis] 2012-05-29 04:40:16 PDT
(In reply to Myk Melez [:myk] [@mykmelez] from comment #50)
> * <a target="_blank"> loads in the browser

On desktop Firefox I assume this means a new tab is opened in a Firefox window?

How could this be handled by B2G's browser app? I'd heard discussions about using an intent/activity for this, but this looks like some kind of protocol handler built into the platform. How would this map onto the user's chosen browser app and how will the browser know to open a new tab to load this URL?

> * <a target="something else"> loads in the runtime

In the same window? Does this apply to all origins?

> * window.open(url, "anything") loads in the runtime
> 
> Note: the behavior of window.open(url, "_blank") is thus to load the URL in
> a new blank window in the runtime.  

What's the UI for handling multiple windows per app in desktop Firefox? I think this would be a non-starter for B2G where there will be no UI to handle multiple windows per app.

> We'll want to do more research to figure
> out if that's the best behavior and then modify it as appropriate, but
> that's another bug.

So this is just for desktop Firefox at the moment?

We have bug #742944 for window.open for mozbrowser iframes on B2G, I guess we need a separate bug for handling anchor targets on B2G as well?
Comment 52 Justin Lebar (not reading bugmail) 2012-05-29 05:01:46 PDT
> How could this be handled by B2G's browser app?

It's a mess, but I think we'll be OK.  I'm planning to catch window.open events at a lower level (nsIWindowWatcher), and I think we'll be able to do our own thing with them.

I admit that I'm not sure.  :)  Honestly it's not even clear to me if B2G apps are causing this code to run.

> I think this would be a non-starter for B2G where there will be no UI to handle multiple windows per 
> app.

The current plan for B2G AIUI is to block window.open(url, "anything") if you already have a window open.  That is, you get exactly one in-app popup.

> I guess we need a separate bug for handling anchor targets on B2G as well?

The window.open work may subsume that work.  I'm not totally sure.
Comment 53 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-05-29 11:25:50 PDT
Rather than checking nodeType/nodeName/hasAttribute, you could just check instanceof HTMLAnchorElement. You could also use target.baseURIObject rather than constructing it yourself, but I don't think you need to worry about that at all - the target.href getter already returns an absolute URI based on the correct base URI, so you should be able to just create a nsIURI directly from that without worrying about the base URI.

An extension to nsIWebBrowserChrome3's onBeforeLinkTraversal might be a cleaner way to do this longer-term, but we don't need to block on that.
Comment 54 Jason Smith [:jsmith] 2012-05-29 14:10:48 PDT
Btw, might be worth building a try build and testing this first before we land it to see if the proposed plan in this patch works for a good amount of top apps.
Comment 55 Myk Melez [:myk] [@mykmelez] 2012-05-29 16:40:57 PDT
Created attachment 628145 [details] [diff] [review]
patch v2: implements gavin's suggestions

(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #53)
> Rather than checking nodeType/nodeName/hasAttribute, you could just check
> instanceof HTMLAnchorElement. You could also use target.baseURIObject rather
> than constructing it yourself, but I don't think you need to worry about
> that at all - the target.href getter already returns an absolute URI based
> on the correct base URI, so you should be able to just create a nsIURI
> directly from that without worrying about the base URI.

All good points, this patch makes the recommended changes.


> An extension to nsIWebBrowserChrome3's onBeforeLinkTraversal might be a
> cleaner way to do this longer-term, but we don't need to block on that.

Yeah, that's probably a better long-term solution.  I've added a comment to that effect.


(In reply to Jason Smith [:jsmith] from comment #54)
> Btw, might be worth building a try build and testing this first before we
> land it to see if the proposed plan in this patch works for a good amount of
> top apps.

The current implementation doesn't work well for a fair number of top apps, so I don't think we need to gate this on such testing, but we should definitely test thoroughly and be prepared to tweak as needed.  I have pushed to tryserver so folks can get a head start on that:

https://tbpl.mozilla.org/?tree=Try&rev=4299bfdd0f78
Comment 56 Jason Smith [:jsmith] 2012-05-30 17:54:25 PDT
Took a look at the try build. This implementation actually works quite well, including with top apps (in fact, it works better than the original implementation). Such things that still worked included:

- Google Accounts Integration
- Browser ID Auth
- The Soundcloud bug 751413

The only thing noted as a problem was setting up pre-auth during an in-app payment, but the marketplace team could probably take care of that issue if they follow the spec of this implementation.

The only thing that's expected was that now the app can continue to run in the web runtime if it goes off the app origin (e.g. click follow us on twitter). However, that will have to be correctly configured by the app itself if it wants the browser to start. As a result - flagging dev-doc-needed - we need this documented somewhere on MDN.
Comment 57 Ed Lee :Mardak 2012-06-01 14:16:37 PDT
kumar, when you're converting in-app payments to use a popup instead of iframes, make sure to use a build with the fix for this bug. Current Nightly builds will redirect the popup into the default browser.

I believe you'll want to do this:

* window.open(url, "anything") loads in the runtime
Comment 58 Myk Melez [:myk] [@mykmelez] 2012-06-07 14:54:56 PDT
Comment on attachment 628145 [details] [diff] [review]
patch v2: implements gavin's suggestions

Hmm, Gavin's review queue is large, while Felipe's is small, and Felipe would make a great reviewer for this code, so switching the review request to him.
Comment 59 :Felipe Gomes (needinfo me!) 2012-06-07 15:12:39 PDT
I can check but asking is probably quicker: what is the current behavior when a webapp tries to navigate away by changing window.location = "...", and will this patch change the behavior? if so, intentionally?
Comment 60 Myk Melez [:myk] [@mykmelez] 2012-06-07 15:41:36 PDT
(In reply to Felipe Gomes (:felipe) from comment #59)
> I can check but asking is probably quicker: what is the current behavior
> when a webapp tries to navigate away by changing window.location = "...",
> and will this patch change the behavior? if so, intentionally?

Currently, the behavior depends on the new value of window.location.  If its origin is the same as the webapp's origin, or its origin is on the hardcoded whitelist, the new value loads in the webapp's window; otherwise, it loads in the browser.

This patch intentionally changes that behavior so that the new value always loads in the webapp's window.
Comment 61 Jason Smith [:jsmith] 2012-06-08 11:46:36 PDT
Some of my testing and bug verifications, most notably bug 749029, is blocked by the implementation of this.
Comment 62 John Drinkwater (:beta) 2012-06-11 03:46:37 PDT
*** Bug 763429 has been marked as a duplicate of this bug. ***
Comment 63 :Felipe Gomes (needinfo me!) 2012-06-11 09:45:32 PDT
Comment on attachment 628145 [details] [diff] [review]
patch v2: implements gavin's suggestions

Review of attachment 628145 [details] [diff] [review]:
-----------------------------------------------------------------

::: webapprt/content/webapp.js
@@ +40,5 @@
> +  document.getElementById("content").removeEventListener("click",
> +                                                         onContentClick,
> +                                                         false, true);
> +}
> +window.addEventListener("unload", onUnload, false);

I don't think we need to worry about removing event listeners on unload. In theory it shouldn't be necessary and would just do unnecessary work on shutdown.

@@ +55,5 @@
> +  let target = event.target;
> +
> +  if (!(target instanceof HTMLAnchorElement) ||
> +      target.getAttribute("target") != "_blank")
> +  {

the rest of this file doesn't use a new line to open the if brace

@@ +71,5 @@
> +    launchWithURI(uri);
> +
> +  // Prevent the runtime from loading the URL.  We do this after directing it
> +  // to the browser to give the runtime a shot at handling the URL if we fail
> +  // to direct it to the browser for some reason.

Do you mean here that the idea is if launchWithURI fails and throws an exception then event.preventDefault won't be called?

Maybe it would be clearer to wrap both in a try-catch like:

try {

  Cc...launchWithURI(uri);
  event.preventDefault();

} catch (e) {}
Comment 64 Myk Melez [:myk] [@mykmelez] 2012-06-11 10:45:34 PDT
Created attachment 631943 [details] [diff] [review]
patch v3: addresses review nits

(In reply to Felipe Gomes (:felipe) from comment #63)
> I don't think we need to worry about removing event listeners on unload. In
> theory it shouldn't be necessary and would just do unnecessary work on
> shutdown.

Ok, removed.


> @@ +55,5 @@
> > +  let target = event.target;
> > +
> > +  if (!(target instanceof HTMLAnchorElement) ||
> > +      target.getAttribute("target") != "_blank")
> > +  {
> 
> the rest of this file doesn't use a new line to open the if brace

I did that to make the opening brace more visible, as it is sometimes hard to spot at the end of a multi-line conditional.  Some other Mozilla code also does this, although other code doesn't, and the style guide doesn't mention it, so I have moved the brace to the end of the second line of the conditional.


> @@ +71,5 @@
> > +    launchWithURI(uri);
> > +
> > +  // Prevent the runtime from loading the URL.  We do this after directing it
> > +  // to the browser to give the runtime a shot at handling the URL if we fail
> > +  // to direct it to the browser for some reason.
> 
> Do you mean here that the idea is if launchWithURI fails and throws an
> exception then event.preventDefault won't be called?

Yes, that's right.


> Maybe it would be clearer to wrap both in a try-catch like:
> 
> try {
> 
>   Cc...launchWithURI(uri);
>   event.preventDefault();
> 
> } catch (e) {}

Hmm, that has the same effect, but it suppresses the exception.  I'd rather let the exception propagate, so at the very least it will be reported.  And either way I'd still want to include the comment, so folks reading the code later will understand that it matters in this case when event.preventDefault() is called.

Here's the updated patch.  Carrying forward review.  This is the version I'll commit.
Comment 65 Myk Melez [:myk] [@mykmelez] 2012-06-11 12:22:53 PDT
Comment on attachment 631943 [details] [diff] [review]
patch v3: addresses review nits

https://hg.mozilla.org/integration/mozilla-inbound/rev/fbad18e20935
Comment 66 Graeme McCutcheon [:graememcc] 2012-06-12 03:05:35 PDT
https://hg.mozilla.org/mozilla-central/rev/fbad18e20935
Comment 67 Jason Smith [:jsmith] 2012-06-18 19:23:25 PDT
Myk - Would it be worth requesting to uplift this patch to Aurora (FF 15)? If so, can you nominate this bug?
Comment 68 Mark Giffin [:markg] 2012-07-15 22:05:10 PDT
I did the developer docs on this here, this will be live soon:
https://developer-new.mozilla.org/en-US/docs/Apps/Apps_architecture#Displaying_pages_in_the_Web_runtime

(That's the new MDN "Kuma" site, ignore end-of-the-world warning)

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