867891 - [Buri][3rdAPP][Facebook]Shared link can not be opened

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect, P1)

Description

[sync-1]

AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.019.085
 Firefox os  v1.0.1
 Mozilla build ID:20130422230201
 
 +++ This bug was initially created as a clone of Bug #448339 +++
 
 Created an attachment (id=401287)
 Shared link1
 
 DEFECT DESCRIPTION:
 (1)Login facebook account, and use facebook app or facebook from everything.me
 (2)For some shared link, press it, nothing happend.--- NOK
 
 Refer to the attached screenshots.
 
  REPRODUCING PROCEDURES:
 
  EXPECTED BEHAVIOUR:
 It should open the link by browser.
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 Medium
  REPRODUCING RATE:
 100%
  For FT PR, Please list reference mobile's behavior:
 
 ++++++++++ end of initial bug #448339 description ++++++++++

Comment 1

[sync-1]

Clone from brother

Comment 2

[sync-1]

Attachment: Shared link2

Clone from brother

Comment 3

[sync-1]

Clone from brother

Comment 4

[sync-1]

Attachment: Shared link1

Clone from brother

Comment 5

[Jason Smith [:jsmith]]

Confirmed - looks like a window.open bug. Happens in the m.facebook.com site in the browser & the app.

Comment 6

[Jason Smith [:jsmith]]

Logcat isn't showing anything interesting.

Comment 7

[Jason Smith [:jsmith]]

Here we go from an example:

<a class="darkTouch" data-sigil="page-transition" target="_blank" href="http://touch.facebook.com/l.php?u=http%3A%2F%2Ft.co%2F3IK5En66Jp&h=pAQEuzP-8&enc=AZP-en0NUMTD9dY-GicPBErwiD4HNi00_mqOF07xN91_uc3cgNy_acf7EuGrmA7FHQMyuksqWe4l65nD8JiX92oG1RbfvMtWsES1Sr9VJuTtACDiQeTNBmwCSfArDcMqBoTz35tyWcHUAbcL-aIZV188&s=1">

The link is a target = blank link. When clicked, these links should be opened in the browser automatically. Apparently this isn't working.

Comment 8

[Jason Smith [:jsmith]]

Hmm...maybe a Browser API bug? Some other DOM bug?

Justin - What do you think?

Comment 9

[Ben Francis [:benfrancis]]

Funny, I'm not able to reproduce this on master or 1.0.1 on a Keon, links open in a new window in the app (the kind with the close button at the top). It's not possible to zoom in and out in this window which is a bit annoying, but it works.

I can't remember which types of anchor target are supposed to open in an overlay window and which are supposed to launch the browser via an activity... Justin can you comment on expected behaviour?

Comment 10

[Justin Lebar (not reading bugmail)]

> I can't remember which types of anchor target are supposed to open in an overlay window and which 
> are supposed to launch the browser via an activity...

I'm pretty sure this is configurable by the app embedder; Gaia can choose what it does by looking at the link target.  You'd have to look to see what it currently does.

If you can reproduce this with debugging enabled in BrowserElementParent.jsm and BrowserElementChildPreload.js (there's a dump statement commented out at the top), I could better say whether this is a problem in Gaia or Gecko.

Comment 11

[Daniel Coloma:dcoloma]

I think we should block on this.

Comment 12

[Andrew Overholt [:overholt]]

(In reply to Ben Francis [:benfrancis] from comment #9)
> Funny, I'm not able to reproduce this on master or 1.0.1 on a Keon, links

I don't see how, but could this be device-specific?

(In reply to Justin Lebar [:jlebar] from comment #10)
> If you can reproduce this with debugging enabled in BrowserElementParent.jsm
> and BrowserElementChildPreload.js (there's a dump statement commented out at
> the top), I could better say whether this is a problem in Gaia or Gecko.

QA, can you get a log as Justin suggests and once you have it, ni?(jlebar) to say where the bug lies?

Comment 13

[Jason Smith [:jsmith]]

(In reply to Andrew Overholt [:overholt] from comment #12)
> (In reply to Ben Francis [:benfrancis] from comment #9)
> > Funny, I'm not able to reproduce this on master or 1.0.1 on a Keon, links
> 
> I don't see how, but could this be device-specific?
> 
> (In reply to Justin Lebar [:jlebar] from comment #10)
> > If you can reproduce this with debugging enabled in BrowserElementParent.jsm
> > and BrowserElementChildPreload.js (there's a dump statement commented out at
> > the top), I could better say whether this is a problem in Gaia or Gecko.
> 
> QA, can you get a log as Justin suggests and once you have it, ni?(jlebar)
> to say where the bug lies?

I need more information about how to turn on debugging here. Is this via a preference? A custom gecko build?

Comment 14

[Justin Lebar (not reading bugmail)]

> If you can reproduce this with debugging enabled in BrowserElementParent.jsm and 
> BrowserElementChildPreload.js (there's a dump statement commented out at the top)

Open up these files, uncomment the commented-out dump statement near the top, rebuild.

Comment 15

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

We don't typically build locally in QA--I'm not sure we even have the latest build instructions for recent targets. 

We can turn this around much faster if you give us the customized build. Otherwise, I'm not sure at what priority we can complete this request.

Comment 16

[Justin Lebar (not reading bugmail)]

This is a tef+ bug, so I hope you can give it the high priority it deserves.  I have other tef+ bugs, so I'm unable to spend cycles spinning builds for QA.

Comment 17

[Jason Smith [:jsmith]]

So the large majority of us don't have a gecko build environment available right now (Naoki & I have Gaia build environments available, but not gecko). Talked with overholt about this. He's already got a build environment ready to go, so he's going to generate the build and we'll use it to get the logs here.

Comment 18

[Andrew Overholt [:overholt]]

Attachment: illustrative screenshots

I cannot reproduce this on my unagi with a local build from today (including uncommenting the dump statements in BrowserElementParent.jsm and BrowserElementChildPreload.js.  Links open fine from what I can tell.

Comment 19

[Andrew Overholt [:overholt]]

Attachment: logcat while clicking on and loading shared link on Facebook

Here's a logcat snippet from when I click on the shared link.  IdleService lines have been removed.

Comment 20

[Andrew Overholt [:overholt]]

Attachment: patch with debugging statements enabled

Reporter, please build with this gecko patch and attach the log here when it fails.

In your B2G tree:

$ cd gecko
$ git apply browserTouchDebug.patch

Comment 21

[Jason Smith [:jsmith]]

Strange. I can reproduce on 5/3/2013 unagi build using a shared link in my facebook.

Comment 22

[Andrew Overholt [:overholt]]

Attachment: links that work, links that don't work

Ah, I think there are different types of links on Facebook.  The attached shows the ones that work for me and the ones that don't.  I can reproduce the inability to click the second type every single time.

I get no output when clicking on them despite them visually changing (blinking, sort of) as I do so.

When I long-press on them, I get prompted to open them in a new tab (in the browser with m.facebook.com; I get nothing in the app), which they do just fine.   I see this while long-pressing (again stripping the IdleService output):

I/Gecko   (  111): BrowserElementChildPreload - Got contextmenu
I/Gecko   (  111): BrowserElementChildPreload - Got contextmenu
I/Gecko   (  111): BrowserElementParent.jsm - fireCtxMenuEventFromMsg: contextmenu [object Object]
I/Gecko   (  111): BrowserElementParent.jsm - fireCtxMenuEventFromMsg: contextmenu [object Object]

Comment 23

[Justin Lebar (not reading bugmail)]

This looks like it could be something wrong in Gecko then.  I'll take a look.

Comment 24

[Justin Lebar (not reading bugmail)]

We get the same behavior in Firefox nightly on my Android phone.

When you click on one of these links on m.facebook.com in desktop Firefox, you eventually get to nsContentUtils::TriggerLink.  One of the arguments there is aClick.  It looks like this indicates whether we're clicking the link or merely hovering our mouse over it.  On desktop Firefox, I get a call with aClick == true, which triggers the opening-a-window codepath.

On my B2G device, when I touch this particular link, we call nsContentUtils::TriggerLink twice, but both times, aClick is false.  Therefore we never handle a click on this link.

On my B2G device, when I click on one of the types of links that works, TriggerLink is also called with aClick == false.  But then the page calls window.open from JS.

Again on my B2G device, when I click on a regular link (at http://example.com), I do see TriggerLink being called with aClick == true, after two calls with aClick == false.

So with this data, this sounds like a bug in Facebook.  Here's a hypothesis: They serve different code to touch and non-touch devices.  They have an onclick handler that cancels all click events -- this would explain why we never see TriggerLink(aClick == true), for either the working or the non-working links.  But then for the one type of link they manually call window.open, and for the other type, they don't.

At this point, I'd normally suggest that it would be evidence in favor of this hypothesis if we tested in another touch-based browser and it behaved like B2G.  But I already spoiled that for you.  :)

Chrome on Android does open both types of links without issue.  But there are a variety of cosmetic bugs in m.facebook.com in Nightly on Android that don't appear in Chrome, which suggests to me that the site is not well-tested in Gecko.

If we want to continue blocking on this, we should find someone who is skilled at understanding what content JS is doing on a remote device.  Or perhaps we can spoof things so we can test this on desktop (at which point I'm still the wrong person for the job).

Comment 25

[Jason Smith [:jsmith]]

Thanks for the thorough analysis. Some followup comments:

(In reply to Justin Lebar [:jlebar] from comment #24)
> We get the same behavior in Firefox nightly on my Android phone.

Hmm. On FxAndroid Nightly, what I saw was that I would get the pop-up warning saying "X site would like to open this content up." If I approve it, then the content will load in a new tab.

Although such functionality doesn't exist on FF OS I think with the pop-up warning.

> 
> So with this data, this sounds like a bug in Facebook.  Here's a hypothesis:
> They serve different code to touch and non-touch devices.  They have an
> onclick handler that cancels all click events -- this would explain why we
> never see TriggerLink(aClick == true), for either the working or the
> non-working links.  But then for the one type of link they manually call
> window.open, and for the other type, they don't.

That hypothesis is right - Facebook serves a touch-optimized site to mobile user agents that Facebook has indicated support for. If Facebook doesn't indicate support for the UA, then a mobile WAP site is used as a fallback.

> 
> Chrome on Android does open both types of links without issue.  But there
> are a variety of cosmetic bugs in m.facebook.com in Nightly on Android that
> don't appear in Chrome, which suggests to me that the site is not
> well-tested in Gecko.

Right. We've been fighting this compatibility battle for a long time with Facebook to get it to function well with gecko, but yeah, there's generally still leftover problems that exist with Facebook when viewed from a gecko mobile browser. Until recently Facebook started giving us the touch optimized site, but we also knew that the touch optimized site came with compatibility problems even after Facebook did a good amount of work to get the site to function on a gecko mobile browser. 

> 
> If we want to continue blocking on this, we should find someone who is
> skilled at understanding what content JS is doing on a remote device.  Or
> perhaps we can spoof things so we can test this on desktop (at which point
> I'm still the wrong person for the job).

I'm thinking we might want to take the outreach option potentially here if this is really a Facebook special case issue - Facebook is a preinstalled app on device, so we can apply pressure to Facebook to indicate that these type of issues need to be fixed to allow the app to remain on device.

If we take the outreach option, we'll need to do the following:

1. Write a short summary of the problem over email that business development can forward over to Facebook
2. Have business development forward the bug report on to Facebook
3. Ask for an ETA on a fix

Unless someone feels strongly we should try to go with the special case bug fix on our side.

Comment 26

[Justin Lebar (not reading bugmail)]

> Hmm. On FxAndroid Nightly, what I saw was that I would get the pop-up warning saying "X site would 
> like to open this content up." If I approve it, then the content will load in a new tab.

Oh, interesting.  Testing again on Android Nightly, it's now opening popups without issue (and without prompting me to block the popup).  I can't reproduce the behavior I'd been seeing before.  :-/

In this case, I think it behooves us to understand

 * whether FB on Android calls window.open for this sort of link, or whether they instead rely on regular link handling (as they do on desktop)

 * if they call window.open in Nightly on Android, why is that not getting called on B2G?

 * if they don't call window.open in Nightly on Android, why is the link click handler not being run on B2G?

In other words, I think this calls for someone to step through the JS that's running in this page in B2G.  If Nightly on Android works properly, I'd probably not kick this off to biz-dev yet, as it could still be a bug in B2G.

Comment 27

[Alex Keybl [:akeybl]]

kward - can we get somebody from Facebook engineering in this bug to aid in the investigation and make sure we're not performing debugging that could easily be replaced with a quick Q/A?

Comment 28

[Karen Ward [:kward]]

Harald, please review with FaceBook engineer(s).  Thanks.

Comment 29

[Karen Ward [:kward]]

Telefonica says ok to disable or fix.

Comment 30

[Jason Smith [:jsmith]]

This is a DOM bug, not a partner apps engineering bug. The DOM team is already looking into this bug.

I already worked with jst & jschoenick on this. jschoenick is working on running this test case with some debugging on to find the root cause.

No need to pull fb engineering in here.

Comment 31

[Johnny Stenback (:jst)]

John is looking into this. This doesn't reproduce on desktop b2g builds, he's spinning a device build right now.

Comment 32

[John Schoenick [:johns]]

Attachment: Add touch events to the popup blocker whitelist

So apparently we never got around to letting touch events past the popup blocker.

On the bright side, the popup blocker works.

Comment 33

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 750787 [details] [diff] [review]
Add touch events to the popup blocker whitelist

I know the code above doesn't use the right coding style, but could you.
if (expr) {
  stmt;
}

And you can probably drop ::

Comment 34

[Justin Lebar (not reading bugmail)]

>  Alex Keybl [:akeybl] 10 minutes ago
> Keywords: checkin-needed

This patch was r+'ed with nits.  Please do not check it in as-is!

Comment 35

[John Schoenick [:johns]]

Attachment: Add touch events to the popup blocker whitelist. r=smaug

Comment 36

[John Schoenick [:johns]]

Comment on attachment 751193 [details] [diff] [review]
Add touch events to the popup blocker whitelist. r=smaug

https://hg.mozilla.org/projects/birch/rev/ccf609dfc31f

Comment 37

[John Schoenick [:johns]]

Attachment: [rebase for b2g18] Add touch events to the popup blocker whitelist. r=smaug

Comment 38

[Ryan VanderMeulen [:RyanVM]]

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

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/c20cad9a676f
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/dbecb0c45504

Comment 40

[Jason Smith [:jsmith]]

Verified on b2g18 on a 5/20 build - I was able to open a shared link to fire the window.open dialog.