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

VERIFIED FIXED in Firefox 16

Status

()

Firefox for Android
Web Apps
P1
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: jsmith, Unassigned)

Tracking

unspecified
Firefox 17
ARM
Android
Points:
---

Firefox Tracking Flags

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

Details

(Whiteboard: [blocking-webrtandroid1+])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Updated

6 years ago
See Also: → bug 752666
(Reporter)

Comment 1

6 years ago
Nominating for k9o for the same reason why the desktop bug was nominated and +ed.
blocking-kilimanjaro: --- → ?
(Reporter)

Updated

6 years ago
OS: Windows 7 → Android
Hardware: x86_64 → ARM
blocking-kilimanjaro: ? → +
(Reporter)

Updated

6 years ago
Blocks: 766259
(Reporter)

Updated

6 years ago
No longer blocks: 766259
(Reporter)

Updated

6 years ago
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
(Reporter)

Updated

6 years ago
QA Contact: aaron.train
(Reporter)

Updated

6 years ago
Blocks: 766259
Priority: -- → P1
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.
Attachment #640846 - Flags: feedback?(mark.finkle)
(Reporter)

Comment 3

6 years ago
(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....
(Reporter)

Updated

6 years ago
status-firefox16: --- → affected
Whiteboard: [qa+]
https://hg.mozilla.org/mozilla-central/rev/4627bfaa6f50
https://hg.mozilla.org/mozilla-central/rev/b9da265c5af3
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
(Reporter)

Updated

6 years ago
Whiteboard: [qa+] → [qa+], [blocking-webrtandroid1+]
Aurora nomination?
Status: RESOLVED → VERIFIED
status-firefox17: --- → verified
(Reporter)

Updated

6 years ago
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+
status-firefox16: affected → fixed
(Reporter)

Updated

6 years ago
status-firefox16: fixed → verified
You need to log in before you can comment on or make changes to this bug.