If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[B2G][Contacts][Facebook] Unable to log out from Facebook in Contacts

VERIFIED FIXED in Firefox OS v1.3

Status

Firefox OS
Gaia::Contacts
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: sarsenyev, Assigned: Jose Manuel Cantera)

Tracking

({regression})

unspecified
1.3 C3/1.4 S3(31jan)
ARM
Gonk (Firefox OS)
regression

Firefox Tracking Flags

(blocking-b2g:1.3+, b2g-v1.2 unaffected, b2g-v1.3 verified)

Details

(Whiteboard: dogfood1.3)

Attachments

(2 attachments)

1.08 MB, video/quicktime
Details
191 bytes, text/html
arcturus
: review+
Details
(Reporter)

Description

4 years ago
Created attachment 8356267 [details]
IMG_0495.MOV

Description:
Cannot log out from Facebook Contacts, doesn't matter sync bar is on or off it doesn't reset data and log in screen doesn't appear, FB just obtaining contacts without asking credentials

Repro Steps:
1) Updated Buri to BuildID: 20140106004001
2) Open Contacts app from the home screen
3) Sync Friends with FB
4) Log in into FB and add some friends
5) Go back to Contact Settings and toggle "Sync friends" off
6) Confirm to remove all Facebook data
7) Toggle FB contacts on

Actual:
A user didn't log out from Facebook contacts

Expected:
The user is logged out from Facebook contacts and user's credentials are required to sign in

Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140106004001
Gaia: 35a60b82f8cf2d759939a350e2dadbb9d8b2f5dc
Gecko: a43cb4b322d3
Version: 28.0a2
Firmware Version: v1.2_20131115

Notes:
Repro frequency: 100%
See attached video
The bug title is different than the bug description here - can you clarify the title here a bit better?
Flags: needinfo?(sarsenyev)
(Reporter)

Comment 2

4 years ago
doesn't' reproduce on the latest 1.2, the log in page pops up everytime when the user toggled "sync friend" bar off and then trying to log in again

Device: Buri 1.2 MOZ
BuildID: 20140106004001
Gaia: 8441587c3b352e052fee07665c21fd192540f19f
Gecko: d552c08a72d0
Version: 26.0
Firmware Version: v1.2_20131115
status-b2g-v1.2: --- → unaffected
status-b2g-v1.3: --- → affected
Flags: needinfo?(sarsenyev)
Summary: [B2G][Contacts][Facebook] The phone doesn't remove Facebook data in Contacts → [B2G][Contacts][Facebook] Unable to log out from Facebook in Contacts
This is particularly bad in the case if the user of the phone ever wants to switch to a different Facebook account.
blocking-b2g: --- → 1.3?
Keywords: regression, regressionwindow-wanted

Updated

4 years ago
QA Contact: mvaughan

Comment 4

4 years ago
This issue started reproducing on the 12/06/13 1.3 build.

- Works -
Environmental Variables:
Device: Buri v1.3 MOZ RIL
BuildID: 20131205040201
Gaia: 1dd0e5c644b4c677a4e8fa02e50d52136db489d9
Gecko: 725c36b5de1a
Version: 28.0a1
Firmware Version: V1.2_US_20131115

- Broken -
Environmental Variables:
Device: Buri v1.3 MOZ RIL
BuildID: 20131206040203
Gaia: 8fca2ca67e8a6022fe6ed8cb576e5d59dfb5237f
Gecko: 1401e4b394ad
Version: 28.0a1
Firmware Version: V1.2_US_20131115
Keywords: regressionwindow-wanted
triage: regression 1.3+
blocking-b2g: 1.3? → 1.3+
(Assignee)

Updated

4 years ago
Assignee: nobody → jmcf
(Assignee)

Comment 6

4 years ago
After debugging this problem, I've observed the following: 

It works perfectly on 1.2 and the logout code has not been changed between v1.2 and v1.3
We invoke Facebook logout service correctly and Facebook responds properly. 

So, I suspect there should be some problem in the local redirection code (the one that captures 302 redirect messages) that do not manage the corresponding cookie reset instructions that come with the HTTP messages sent by Facebook. And this seems to be a Gecko regression between 1.2, 1.3 and the following versions. 

ni Fabrice as he is the owner of the local redirection code and can give us more cues about what could be happening
Flags: needinfo?(fabrice)
(Assignee)

Comment 7

4 years ago
I've observed the following error appears occasionally on the debug traces:

[JavaScript Error: "TypeError: url.searchParams is undefined" {file: "about:neterror?e=fileNotFound&u=app%3A//communications.gaiamobile.org/contacts/null&c=UTF-8&d=Firefox%20can%27t%20find%20the%20file%20at%20app%3A//communications.gaiamobile.org/contacts/null.&m=app%3A//communications.gaiamobile.org/manifest.webapp" line: 568}]

I don't know if that has to do with this bug
(In reply to Jose M. Cantera from comment #7)
> I've observed the following error appears occasionally on the debug traces:
> 
> [JavaScript Error: "TypeError: url.searchParams is undefined" {file:
> "about:neterror?e=fileNotFound&u=app%3A//communications.gaiamobile.org/
> contacts/null&c=UTF-8&d=Firefox%20can%27t%20find%20the%20file%20at%20app%3A//
> communications.gaiamobile.org/contacts/null.&m=app%3A//communications.
> gaiamobile.org/manifest.webapp" line: 568}]
> 
> I don't know if that has to do with this bug

Aus - I recall you looking into something like this recently with the regressions with the new offline error page. Could this error be triggering this bug?
Flags: needinfo?(aus)
There have been no changes in the redirection code since it landed basically. If it's not a front end issue, we should get a 1 day regression range to check the gecko side.
Flags: needinfo?(fabrice)
(In reply to Fabrice Desré [:fabrice] from comment #9)
> There have been no changes in the redirection code since it landed
> basically. If it's not a front end issue, we should get a 1 day regression
> range to check the gecko side.

Here's a push log with the window above.

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=725c36b5de1a&tochange=1401e4b394ad

Comment 11

4 years ago
jsmith - This should be fixed now thanks to this commit: https://github.com/mozilla-b2g/gaia/commit/86229eb4f6ba35b2dbd1682b812c6f747bbe6f38
Flags: needinfo?(aus)
(In reply to Ghislain 'Aus' Lacroix from comment #11)
> jsmith - This should be fixed now thanks to this commit:
> https://github.com/mozilla-b2g/gaia/commit/
> 86229eb4f6ba35b2dbd1682b812c6f747bbe6f38

Okay - in tomorrow's build, can we check if this still reproduces on master?
Keywords: qawanted
(Assignee)

Comment 13

4 years ago
(In reply to Ghislain 'Aus' Lacroix from comment #11)
> jsmith - This should be fixed now thanks to this commit:
> https://github.com/mozilla-b2g/gaia/commit/
> 86229eb4f6ba35b2dbd1682b812c6f747bbe6f38

with the referred commit applied the same error appears on the console 

E/GeckoConsole( 1501): [JavaScript Error: "TypeError: url.searchParams is undefined" {file: "about:neterror?e=fileNotFound&u=app%3A//communications.gaiamobile.org/contacts/null&c=UTF-8&d=Firefox%20can%27t%20find%20the%20file%20at%20app%3A//communications.gaiamobile.org/contacts/null.&m=app%3A//communications.gaiamobile.org/manifest.webapp" line: 568}]

Updated

4 years ago
Keywords: qawanted
(Assignee)

Comment 14

4 years ago
Fabrice,

This error also appears on the console when logout is done 

E/GeckoConsole( 1501): [JavaScript Error: "[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/FormHistory.jsm :: <TOP_LEVEL> :: line 376"  data: no]" {file: "resource://gre/modules/FormHistory.jsm" line: 376}]

any idea?
(Assignee)

Updated

4 years ago
Flags: needinfo?(fabrice)
(In reply to Jose M. Cantera from comment #14)
> Fabrice,
> 
> This error also appears on the console when logout is done 
> 
> E/GeckoConsole( 1501): [JavaScript Error: "[Exception... "Component returned
> failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult:
> "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
> resource://gre/modules/FormHistory.jsm :: <TOP_LEVEL> :: line 376"  data:
> no]" {file: "resource://gre/modules/FormHistory.jsm" line: 376}]
> 
> any idea?

If the process 1501 is not the parent process (check what the pid is for b2g in b2g-info), then it's not surprising to see that code fail. But we should not run it at all, so it will be worth filing. I did not see that when I tried to reproduce this bug though (I can repro the "not logged out" bug).
Flags: needinfo?(fabrice)
I've been logging the traffic that gets send to Facebook, and there's a big difference between 1.2 and 1.3. 

a) Login is done on a window (window.open(...)) inside of the contacts app. As part of the login process, Facebook sets a bunch of cookies.
b) Logout is done via a systemXHR, on the contacts app.

On 1.2, when the logout URL is loaded, the bunch of cookies set on step a) are sent back to Facebook. Facebook answer includes a 'delete cookie' for each of those cookies. On 1.3, though, when then logout URL is loaded, the bunch of cookies is *not* sent. Since Facebook doesn't get any cookie, it doesn't erase them either, and thus the answer doesn't get any 'delete cookie' (not that it would matter).

The next time a facebook page is loaded via window.open, the bunch of cookies is sent again (and thus the user auto logs in again). Actually, the XHR is not saving the cookies nor sending them.

I think this was introduced in bug 927196 (which I can't access, actually, so I don't know what the rationale was), concretely on:

http://hg.mozilla.org/mozilla-central/diff/bcda0491338b/dom/workers/XMLHttpRequest.cpp

Now every time mozSystem is set, mozAnon is also automatically set and thus the cookies are not sent, and thus the session is not actually closed.
I've been talking with José Manuel, and we see two ways forward here. One is do the logout on a window also (which is the way it was originally, and was changed because it was bad UX). And the other one is revert the change that forces mozAnon when mozSystem is set.

I still don't get why mozSystem implies mozAnon (although I know that's the way it was documented originally), considering that mozSystem is behind a privileged permission, and that each apps have their own cookie jar. I thought the idea of mozSystem was... well, allowing to to what we were doing here.

As the reviewer for the XHR patch, what do you think, Jonas?
Flags: needinfo?(jonas)
(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #17)
> I've been talking with José Manuel, and we see two ways forward here. One is
> do the logout on a window also (which is the way it was originally, and was
> changed because it was bad UX). And the other one is revert the change that
> forces mozAnon when mozSystem is set.

We can't revert the change in bug 927196. That was implemented for security reasons. I'll cc you on the bug to allow you to get more details on this.

Updated

4 years ago
Blocks: 927196

Updated

4 years ago
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
(Assignee)

Comment 19

4 years ago
then we will fix the bug by logging out by opening a window and then closing it. the UX will be a bit weird but I think we don't have alternative
(Assignee)

Comment 20

4 years ago
Created attachment 8362836 [details]
15548.html

this patch implements logout by opening a window as per comment #16
Attachment #8362836 - Flags: review?(francisco.jordano)
Comment on attachment 8362836 [details]
15548.html

Perfect solution, so clean and removing a lot of code.

Tried on the phone and working smoothly.

Thanks Jose for the perfect work!
Attachment #8362836 - Flags: review?(francisco.jordano) → review+
(Assignee)

Comment 22

4 years ago
https://github.com/mozilla-b2g/gaia/commit/b9e2e219ab2514b03b25cb37a6434ef8f116fd4d
Flags: needinfo?(jonas)
(Assignee)

Updated

4 years ago
Keywords: verifyme
Should this be closed resolved fixed with the landing in comment 22?
(Assignee)

Comment 24

4 years ago
(In reply to Jason Smith [:jsmith] from comment #23)
> Should this be closed resolved fixed with the landing in comment 22?

yes
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Uplifted b9e2e219ab2514b03b25cb37a6434ef8f116fd4d to:
v1.3: 46bfabcec36899813976012f2697a225859b835f
status-b2g-v1.3: affected → fixed
This issue no longer reproduces in the latest v1.3. When the user de-syncs from their facebook they are properly logged out and prompted to log in again once they decide to re-sync.

Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140123004001
Gaia: 6fbeac2415f07f10de181f0877ddf67ee299b885
Gecko: e8f6bdf8db3d
Version: 28.0a2
Firmware Version: V1.2-device.cfg
Status: RESOLVED → VERIFIED
status-b2g-v1.3: fixed → verified
Since it has already been verified, removing verifyme tag.
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.