Closed Bug 936250 Opened 6 years ago Closed 6 years ago

[B2G][Contacts] Canceling FB sync results in the contacts app being unresponsive.


(Firefox OS Graveyard :: Gaia::Contacts, defect)

Gonk (Firefox OS)
Not set


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

1.3 C1/1.4 S1(20dec)
blocking-b2g 1.3+
Tracking Status
b2g-v1.2 --- unaffected
b2g-v1.3 --- fixed


(Reporter: laliaga, Assigned: gtorodelvalle)



(Keywords: regression)


(1 file)

After entering in your Facebook crendentials for contacts sync, while it looks up your friends list if you tap cancel, the contacts app becomes unresponsive. Only after force closing the app through task manager/card view will the contacts app become responsive again.

Repro Steps:
1. Update Buri device to 1.3 buildID: 20131107040200.
2. Launch Contacts > Settings > Facebook Sync toggle.
3. Enter appropriate FB credentials and tap log in.
4. While it's looking up the Friends List, tap cancel.

Actual Results:
Contacts app is unresponsive.

Expected Results:
Contacts app to always be responsive after any destructive action.

Repro Rate: 5/5 100%

Environmental Variables:
Gaia   42bbe26a72e030faf07a6fc297f61a3a8ccda25b
SourceStamp 70de5e24d79b
BuildID 20131107040200
Version 28.0a1

See attached: logcat
Add'l Notes: Does not occur 1.2
blocking-b2g: --- → koi?
blocking-b2g: koi? → 1.3?
QA Contact: mvaughan
This issue started reproducing on the 9/25 1.3 build.

- Works -
Environmental Variables:
Device: Buri v1.3 Mozilla RIL
BuildID: 20130924040201
Gaia: a22ba4a3a9efd99f94adf9ece8a2b391d4df295b
Gecko: 1fda74e33e06
Version: 27.0a1

- Broken -
Environmental Variables:
Device: Buri v1.3 Mozilla RIL
BuildID: 20130925174530
Gaia: 7e42b4d690049709c62e8783910f16ab20869f42
Gecko: fa0e6916f88c
Version: 27.0a1
triage: 1.3+ regression
blocking-b2g: 1.3? → 1.3+
Assignee: nobody → gtorodelvalle
Attached file 14182.html
Attachment #8339985 - Flags: review?(jmcf)
Comment on attachment 8339985 [details]

r- as per the comments on GH. Particularly 

E/GeckoConsole( 2802): [JavaScript Error: "SyntaxError: An invalid or illegal string was specified" {file: "app://" line: 87}]

In addition I would feel more confortable if the patch only modifies the module/s affected by what is reported in the bug. nothing more nothing less. 

Attachment #8339985 - Flags: review?(jmcf) → review-
Although you are right, in this case I think that the additional refactoring was justified :-) In fact, as you will see in the PR the problem was due to a bad brackets closing at Anyhow, this update is still needed ;-)

Anyhow, as you say, I think it is safer to solve the problem the bug reports and if necessary open a new one for the additional refactoring. Consequently, I removed any changes to contacts_matching_controller.js since not affected by this bug :-)

Ready for a new review ;-)

Attachment #8339985 - Flags: review- → review?(jmcf)
Comments included in New verdict needed ;-) Thanks!
Comment on attachment 8339985 [details]

provided the indentation nits are fixed

Thanks, Germán
Attachment #8339985 - Flags: review?(jmcf) → review+
Duplicate of this bug: 938602
Merged in master:
Closed: 6 years ago
Resolution: --- → FIXED
Depends on: 949197
This needs to be backed out - this caused a smoketest regression in bug 949197.

Can you back this out & reland with the fix needed for bug 949197?
Flags: needinfo?(gtorodelvalle)
Whiteboard: [NO_UPLIFT]
Hi Jason, we are currently investigating if this bug is the real cause of the issue :-)
Flags: needinfo?(gtorodelvalle)
Target Milestone: --- → 1.3 C1/1.4 S1(20dec)
as we have said in the related bugs, this has nothing to do and it will not be backed out
(In reply to Jose M. Cantera from comment #12)
> as we have said in the related bugs, this has nothing to do and it will not
> be backed out

That's incorrect. See which explains that this is clearly the cause of this regression. The underlying problem could be a gecko bug under the hood, but this patch clearly triggered the code path to reveal the gecko bug. This needs to be backed out until the underlying gecko problem is resolved or we apply a Gaia workaround.

John - Can you back this out?
Flags: needinfo?(jhford)
Discussed with Ben about some of the points above - I'm going to verify if the backout of this patch resolves the smoketest regression.
Flags: needinfo?(jhford) → needinfo?(jsmith)
No longer depends on: 949197
Looks like upon more digging that regression here happened in the JS engine per bug 697343. This is now safe to uplift.
Flags: needinfo?(jsmith)
Whiteboard: [NO_UPLIFT]
Uplifted 6e59c3cd7a793303695521e387c9ce09c2ecb7aa to:
v1.3: abd571f4e567d4647c8b9c59f85ca293ffcd6ebf
Duplicate of this bug: 951949
You need to log in before you can comment on or make changes to this bug.