853456 - target=_blank doesn't work with <img> links

Status: VERIFIED FIXED

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

Description

[Andrew Williamson [:eviljeff]]

from an email:

it seems that there is a bug that only affects webapps on Android, whereby hyperlinks that contain an img do not open externally. 

Works (opens externally):
<div class="image"><a href="{href}" target="_blank">{text}</a></ div>

Doesn't work (opens internally):
<div class="image"><a href="{href}" target="_blank"><img src="{src}"/></a></div>

This issue only appears to affect the app runtime on Android, as it works fine on the Firefox beta browser on Android and on Firefox OS full screen web apps.

Comment 1

[Jason Smith [:jsmith]]

Putting on tracking cause this breaks compat across apps on platforms and touches on target=blank functionality, which is important for opening content outside of the app.

Comment 2

[Andrew Williamson [:eviljeff]]

Added a URL to an app that exhibits this issue.  Try the video links.

Comment 3

[Jason Smith [:jsmith]]

This is compatibility bustage in comparison to FxOS.

==> P1

Comment 4

[Jason Smith [:jsmith]]

Explanation is needed here.

This was marked tracking on web apps initiative, so I do not understand why this is not being tracked on the fennec side.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

It is being tracked for web apps, there for does not need to be tracked for fennec

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch

This walks the tree to find links rather than just using event.target.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 739885 [details] [diff] [review]
Patch

>+    while(target && !(target instanceof HTMLAnchorElement)) {

nit: add a space
  while (

>     let uri = Services.io.newURI(target.href,
>                                  target.ownerDocument.characterSet,
>                                  null);

Not your change, but could you make this all one line?

Comment 8

[Wesley Johnston (:wesj)]

Forgot about the nits. Wil push a follow up.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3c0e77a706f4

Comment 9

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef755a443f3c

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3c0e77a706f4

Comment 11

[:Cykesiopka]

Looks like the m-i commit for the follow up had the wrong bug number.
Anyways, merged to m-c here:

https://hg.mozilla.org/mozilla-central/rev/ef755a443f3c