Closed Bug 766802 Opened 12 years ago Closed 12 years ago

Clicking target=blank links in a web app should load those links in the browser

Categories

(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P1)

ARM
Android
defect

Tracking

(blocking-kilimanjaro:+, firefox16 verified, firefox17 verified)

VERIFIED FIXED
Firefox 17
blocking-kilimanjaro +
Tracking Status
firefox16 --- verified
firefox17 --- verified

People

(Reporter: jsmith, Unassigned)

References

Details

(Whiteboard: [blocking-webrtandroid1+])

Attachments

(1 file)

Right now if I'm in a web app, it acts as a complete navigable view for going around the app, but there's no way to allow the user to exit the app back to the browser on clicking certain links that go to external sites off of the app origin. Instead, we end up loading that link directly in the web view, resulting in the app losing context of where its origin is. We should adapt some policy for when a link opens in the browser vs. opens within the app.

Desktop following a policy where a link that uses target equals blank opens the link in the browser, not the app. All other links open within the app. This requires the app do the work manually to determine whether the off-origin link loads within the app or goes to the browser. But I think the option should be followed on firefox for android as well with web apps.

See bug 752666 for more information for what desktop did.
See Also: → 752666
Nominating for k9o for the same reason why the desktop bug was nominated and +ed.
blocking-kilimanjaro: --- → ?
OS: Windows 7 → Android
Hardware: x86_64 → ARM
blocking-kilimanjaro: ? → +
No longer blocks: Blocking-FFA-WebRT1+
Summary: Need some way to be able to allow links in a web app to go the browser in some cases → Clicking target=blank links in a web app should load those links in the browser
QA Contact: aaron.train
Priority: -- → P1
Attached patch WIP PatchSplinter Review
Grr... I've been trying forever to find an app that uses this. I need a break. But I think this will work. Probably some naming things you'll hate, and I added a "webapp" window argument in addition to "pinned". I want to remove pinned, but wasn't sure if it served some purpose.
Attachment #640846 - Flags: feedback?(mark.finkle)
(In reply to Wesley Johnston (:wesj) from comment #2)
> Created attachment 640846 [details] [diff] [review]
> WIP Patch
> 
> Grr... I've been trying forever to find an app that uses this. I need a
> break. But I think this will work. Probably some naming things you'll hate,
> and I added a "webapp" window argument in addition to "pinned". I want to
> remove pinned, but wasn't sure if it served some purpose.

Clicking news items in the Times Crossword app is a good example of a target=_blank use case in-app.

https://marketplace.mozilla.org/en-US/app/the-times-crossword-beta/

Btw, feel free to ask me directly if you need to find an app that does X piece of functionality. I've looked at lots of these during desktop testing and app reviews, so I probably could point you to an app that demonstrates some piece of functionality.
Attachment #640846 - Flags: feedback?(mark.finkle) → review?(mark.finkle)
Comment on attachment 640846 [details] [diff] [review]
WIP Patch


>diff --git a/mobile/android/chrome/content/WebAppRT.js b/mobile/android/chrome/content/WebAppRT.js

>+/*
>+ * This code is heavily based on Arc90's readability.js (1.7.1) script
>+ * available at: http://code.google.com/p/arc90labs-readability
>+ *
>+ * readability.js is licensed under the Apache License, Version 2.0
>+ * Copyright (c) 2010 Arc90 Inc
>+**/

I bet it's not!

>+var WebAppRT = {
>+  init: function() {
>+    this.deck = document.getElementById("browsers");
>+    this.deck.addEventListener("click", onContentClick, false, true);

Just add a "handleEvent" method and do it all in WebAppRT. No need to make a onContentClick
>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

>+    let webapp = false;

>+      if (window.arguments[5])
>+        webapp = window.arguments[5];

>+    if (webapp)
>+      WebAppRT.init();

Just use "pinned" for now and reduce the size of this patch :)

Don't add "webapp" to BrowserCLH either. If we want to do that, we can do it in a separate patch/bug.
Attachment #640846 - Flags: review?(mark.finkle) → review+
I must be sleepy, but I managed to push this with no commit message...
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c49724bcbfd

Not sure how the sheriffs will figure this one out....
Whiteboard: [qa+]
https://hg.mozilla.org/mozilla-central/rev/4627bfaa6f50
https://hg.mozilla.org/mozilla-central/rev/b9da265c5af3
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Whiteboard: [qa+] → [qa+], [blocking-webrtandroid1+]
Aurora nomination?
Status: RESOLVED → VERIFIED
Whiteboard: [qa+], [blocking-webrtandroid1+] → [blocking-webrtandroid1+]
Comment on attachment 640846 [details] [diff] [review]
WIP Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Webapps
User impact if declined: Webapp links will always open in the app
Testing completed (on m-c, etc.): Landed on mc two weeks ago
Risk to taking this patch (and alternatives if risky): Low risk. Webapps only. Needed for almost every other webapp patch.
String or UUID changes made by this patch: None.
Attachment #640846 - Flags: approval-mozilla-aurora?
Comment on attachment 640846 [details] [diff] [review]
WIP Patch

mobile webapps only, needs more users testing, approving for Aurora.
Attachment #640846 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: