Closed Bug 962774 Opened 10 years ago Closed 10 years ago

clicking shared links in facebook opens the link in two duplicate tabs

Categories

(Tech Evangelism Graveyard :: Preinstalled B2G Apps, defect, P2)

ARM
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: dbaron, Assigned: colby)

References

()

Details

(Keywords: regression, Whiteboard: [systemsfe])

Attachments

(1 file)

Steps to reproduce:
 1. open Gaia browser app
 2. load https://m.facebook.com/ and log in if you aren't already
 3. find a shared link in the newsfeed
 4. click on the page's title to open it
    (note: there might be a URL in the text; clicking that doesn't show the bug)

Actual results:  The link opens *twice* -- there are two new tabs opened in the browser.

Expected results:  The link opens once -- just one new tab opened.

I'm seeing this in a self-built v1.3 build on a hamachi.  It did not occur in v1.2.
In this example, the bug:

 * does occur when tapping on "The Battle in Kiev:  Two Killed in Ukraine Protest"

 * does NOT occur when topping on "http://t.co/E7PQlyk4ZD"
Whiteboard: regression → regression, [systemsfe]
Ben, Dale, any idea what's going on here?
I'm not able to reproduce this on a recent 1.3 build on Keon. Waiting for regression window.
Switching to QA Wanted to check if we can reproduce this on the latest 1.3 build.
Whiteboard: regression, [systemsfe] → [systemsfe]
QA Contact: sarsenyev
I was able to reproduce it on the latest 1.3 build.
A link opens twice when I tapped the image with URL provided.
For the test I used a couple different links along with the provided "http://t.co/E7PQlyk4ZD"

Device: Buri 1.3 MOZ
BuildID: 20140123004001
Gaia: 6fbeac2415f07f10de181f0877ddf67ee299b885
Gecko: e8f6bdf8db3d
Version: 28.0a2
Firmware Version: v1.2-device.cfg
Keywords: qawanted
Okay - can you check if this reproduces on 1.1?
Keywords: qawanted
It doesn't reproduce on 1.1

Device: Buri 1.1 MOZ
BuildID: 20140123041201
Gaia: c434fe9a0e823029796805e141cfa983cda2d246
Gecko: aa0ceb07a73e
Version: 18.0
Firmware Version: v1.2-device.cfg
Keywords: qawanted
The regression window is a little wide.
10/14 has too builds, I wasn't able to test on later 10/14 build during the Facebook webpage didn't allow to put login credentials, the fields are blocked even after resetting, restarting, reflashing

Last working build where is no duplicated links:
Device: Buri 1.3 MOZ
BuildID: 20131014040204
Gaia: f240abf567325d486c3d0434791b1d132cfa327a
Gecko: 211337f7fb83
Version: 27.0a1
Firmware Version: v1.2-device-cfg

The build where is FB login fields are blocked and couldn't test it:
Device: Buri 1.3 MOZ
BuildID: 20131014080339
Gaia: 74e7e69d98206bda5c517c0ada57d43de662913e
Gecko: ab8e70fb76a8
Version: 27.0a1
Firmware Version: v1.2-device.cfg

The first broken build with duplicated links:
Device: Buri 1.3 MOZ
BuildID: 20131015040202
Gaia: 17e871ae1f82699793e3cd28acda805ba724a8b6
Gecko: febfe3c7732b
Version: 27.0a1
Firmware Version: v1.2-device.cfg
blocking-b2g: 1.3? → 1.3+
Assignee: nobody → tclancy
I've noticed something interesting. For me, I see the bug if I use a Release build, but not if I use a Debug build. (I built each manually, and I'm testing on a Hamachi.)

I wonder what would be different between Debug and Release. 

Could this be timing related?
(In reply to Ted Clancy [:tedders1] from comment #9)
> I've noticed something interesting. For me, I see the bug if I use a Release
> build, but not if I use a Debug build. (I built each manually, and I'm
> testing on a Hamachi.)
> 
> I wonder what would be different between Debug and Release. 
> 
> Could this be timing related?

Probably timing related :( Or do you see different assertions in logcat?
Okay, I think I can see what's happening.

When the user taps, Gonk generates both NS_TOUCH_START/NS_TOUCH_END messages and NS_MOUSE_BUTTON_DOWN/NS_MOUSE_BUTTON_UP messages.

The NS_TOUCH_START/NS_TOUCH_END messages get forwarded from the parent B2G process to the Browser process. The NS_MOUSE_BUTTON_DOWN/NS_MOUSE_BUTTON_UP messages are sent to the AsyncPanZoomController, which then sends a "HandleSingleTap" message to the Browser process.

When the Browser process receives the NS_TOUCH_END, it executes some Javascript code on the Facebook page, which opens the link. Then the Browser process receives the "HandleSingleTap" message, which triggers a NS_MOUSE_CLICK message, which executes some more Javascript code on the Facebook page, which opens the link again.

So the link gets opened twice. (Sometimes the link is only opened once. The messages from the B2G process are sent to the Browser process asynchronously, so it depends on timing. Sometimes the new page becomes the "focused" page before the second signal arrives.)

The problem is, I don't know what the proper behaviour should be, so I don't know what I should do to fix this. I might need to talk to someone about what the proper behaviour should be in this case.
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
Whiteboard: [systemsfe] → [systemsfe] p=3
I've consulted with :kats, and this appears to be an issue with Facebook. They shouldn't be opening links on both Touch events and Click events.

I'm assigning this to the Tech Evangelism team so they can reach out to Facebook.
Component: Gaia::Browser → Other
Product: Firefox OS → Tech Evangelism
Whiteboard: [systemsfe] p=3
Target Milestone: 1.3 C3/1.4 S3(31jan) → Jan
Version: unspecified → Trunk
Assignee: tclancy → other
(In reply to Ted Clancy [:tedders1] from comment #12)
> I've consulted with :kats, and this appears to be an issue with Facebook.
> They shouldn't be opening links on both Touch events and Click events.
> 
> I'm assigning this to the Tech Evangelism team so they can reach out to
> Facebook.

Actually that's not right - if the issue hasn't reproduced in a past release, then that means it's on our end to resolve the problem. That might mean this is a problem with how we prioritize event handling in the DOM, so I'm sending this over to DOM: Events to look more.
Assignee: other → nobody
Component: Other → DOM: Events
Product: Tech Evangelism → Core
Target Milestone: Jan → ---
Version: Trunk → 28 Branch
If the issue wasn't present in past releases, I suspect that's due to timing differences. No one should be relying on timing differences.
Actually, ignore me. Letting the DOM Events team review this is probably a good idea.
I don't see any suspicious changes to event handling 
in http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=211337f7fb83&tochange=febfe3c7732b
Bug 915757 landed, but was then also backed out, so it shouldn't have affected.

Could gaia change Bug 925289 have caused this?
(In reply to Olli Pettay [:smaug] from comment #16)
> I don't see any suspicious changes to event handling 
> in
> http://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=211337f7fb83&tochange=febfe3c7732b
> Bug 915757 landed, but was then also backed out, so it shouldn't have
> affected.
> 
> Could gaia change Bug 925289 have caused this?

Don't think so - that bug deals with a desktop extension, which should not affect phone behavior.
(In reply to Olli Pettay [:smaug] from comment #17)
> Oh, perhaps Bug 922896.

Dale is going to investigate this.
Component: DOM: Events → Panning and Zooming
Assigning based on comment 19.

FWIW it still sounds like a timing issue to me. It's quite possible that this worked fine pre-APZ because if they loaded a page in their touchend handler then the click event would probably get dropped as the click detection happened in the content process. With APZ the click detection happens in the parent process and so might still be running while the content process is fetching the new page.
Assignee: nobody → dale
Whiteboard: [systemsfe]
Yeh trying to get a working build to very now, but it we are sending a touch / mouse event and this was purely a timing issue,  I think it will still have to be evangelism, I dont think we can revert system wide performance improvements due to buggy web content right?
So I disabled the optimisation in https://bugzilla.mozilla.org/show_bug.cgi?id=922896 and couldnt reproduce the problem after that

However as mentioned, we are sending the correct events to content (http://patrickhlauke.github.io/touch), its a bug in facebooks javascript to react to both touch and mouse events, a rather large one and I dont think we should be regressing performance here
Okay, it sounds like we have a decision to make here.

According to what I’ve heard from :kats, :smaug, and :daleharvey, it’s wrong for Facebook to respond to both Touch and Click events. That could be considered the root cause of this problem.

However, the problem wasn’t visible until we removed the 300ms delay between Touch and Click events (Bug 922896). That was a change made for better performance.

Our options are:
1) Do nothing, but let Facebook know they have an issue (through our Platform Evangelism team); or

2) Revert Bug 922896, let Facebook know they have an issue, and then re-apply Bug 922896 at a future date after Facebook fixes their issue. (After the 1.4 release, most likely.) In the meantime, our performance will be worse. There’s also no guarantee that Facebook will ever fix their issue. 

To some degree, this is a question of what’s more important: performance or working around a Facebook glitch.

I'm going to take this to our Product Manager.
I don't think we should be taking a performance hit in favor of working around a bug Facebook has. We do have a locked down partner connection with them, so I think a TE approach is the best path forward at this point.

Karen - Can you coordinate with FB to fix this issue?
Assignee: dale → nobody
Blocks: b2g-facebook
blocking-b2g: 1.3+ → ---
Component: Panning and Zooming → Preinstalled B2G Apps
Flags: needinfo?(kward)
Product: Core → Tech Evangelism
QA Contact: sarsenyev
Version: 28 Branch → unspecified
Colby, please review and let us know when we can expect a fix.  Thank you.
Assignee: nobody → colby
Status: NEW → ASSIGNED
Flags: needinfo?(kward)
Priority: -- → P2
Hmm... This is definitely an issue on our end. I'll look into it and report back.
This seems to have stopped happening for me sometime earlier this week.
The fix for this was released last week.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
Product: Tech Evangelism → Tech Evangelism Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: