Closed Bug 1183301 Opened 9 years ago Closed 9 years ago

[B2G] Can't open a new attention window

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.5+, b2g-v2.2r affected, b2g-master verified)

VERIFIED FIXED
FxOS-S3 (24Jul)
blocking-b2g 2.5+
Tracking Status
b2g-v2.2r --- affected
b2g-master --- verified

People

(Reporter: dietrich, Assigned: alwu)

References

Details

(Keywords: regression, Whiteboard: [dogfood-blocker])

Attachments

(2 files, 1 obsolete file)

STR:

1. open contacts -> preferences -> import
2. choose either Gmail or Outlook

Expected: import process begins

Actual: nothing happens, and the UI doesn't respond. have to kill the app to get it working again.
[Blocking Requested - why for this release]: Mayor regression

I am also reproducing this bug in Flame with latest master build (7/14) but with yesterday's master build (7/13), all was working as expected.

We have reviewed the commits in Contacts application and neither of them are causing this issue, so it seems that could have been introduced by a Gecko patch and it's not related to the NGA work in Contacts application.

The issue is also reproducing in the FTU, and once it happens, the buttons do not respond so it's necessary to kill the Contacts application or restart the device (if the issue is reproduced during the FTU)

Environmental variables (7/14) NOT working:
flame master (2.5.0 version)
Build ID: 20150714060818
Gecko: c7cb3ca
Gaia: 66638d0
Platform version: 42.0a1


Environmental variables (7/13) working fine:
flame master (2.5.0 version)
Build ID: 20150713
Gecko: 9d932ee
Gaia: 167a22b
Platform version: 42.0a1
blocking-b2g: --- → 2.5?
Summary: [aries] import of Gmail/Outlook contacts is broken and hoses the app → [aries][Flame] import of Gmail/Outlook contacts is broken and hoses the app
Kevin, does this bug block the smoketest?
Flags: needinfo?(ktucker)
In order to close this bug, a regression test should be added to make sure this cannot happen again without warning.
Flags: needinfo?(ktucker)
QA Contact: ktucker
We don't have any smoke test cases regarding importing from Outlook or Gmail only from the SIM in the FTU so I guess it is not a smoke test blocker but this is certainly a critical issue.
Mozilla Inbound

Last Working
Environmental Variables:
Device: Flame 2.5
BuildID: 20150710163851
Gaia: e4b63559eba364892867eb381c3002d6518e5d6a
Gecko: 07bcf36f5ab2
Version: 42.0a1 (2.5)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

First Broken
Environmental Variables:
Device: Flame 2.5
BuildID: 20150710170452
Gaia: e4b63559eba364892867eb381c3002d6518e5d6a
Gecko: 675ea719b91c
Version: 42.0a1 (2.5)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Last Working Gaia First Broken Gecko: Issue DOES reproduce
Gaia: e4b63559eba364892867eb381c3002d6518e5d6a
Gecko: 675ea719b91c

First Broken Gaia Last Working Gecko: Issue DOES NOT reproduce
Gaia: e4b63559eba364892867eb381c3002d6518e5d6a
Gecko: 07bcf36f5ab2

Gecko pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=07bcf36f5ab2&tochange=675ea719b91c

This looks to have been caused by bug 1113086.
Blocks: 1113086
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
NI on author and reviewer for commits in bug 1113086, looks like it caused this severe issue due to the found window.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
Flags: needinfo?(amarchesini)
Flags: needinfo?(alwu)
Reproduced the issue with logcat running (included 'Gaia Debug Traces')

Device: Aries 2.5
BuildID: 20150714110444
Gaia: 66638d0e65bf58b7f640bcc7bed4a0b23d1356c6
Gecko: e786406bc683
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0
Attached patch Not throw error (obsolete) — Splinter Review
This issue might be due to we throw the exception, so broke the code in app_window.js. 

We throw the exception because the pop-up window didn't have the frameloader. However, I don't know that is a bug or dom design.

This patch can solve this issue, but I can't sure it's correct way.

---

Hi, Baku,
Could you help me check this patch?
Thanks!
Flags: needinfo?(alwu)
Attachment #8633890 - Flags: feedback?(amarchesini)
Blocks: 1181982
Need another better way.
Flags: needinfo?(amarchesini)
Assignee: nobody → alwu
Comment on attachment 8633890 [details] [diff] [review]
Not throw error

Review of attachment 8633890 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/html/nsBrowserElement.cpp
@@ +522,5 @@
>  
>    // If empty, it means that this is the first call of this method.
>    if (mBrowserElementAudioChannels.IsEmpty()) {
>      nsCOMPtr<nsIFrameLoader> frameLoader = GetFrameLoader();
>      if (!frameLoader) {

ok! we can do it but write this:

if (NS_WARN_IF(!frameLoader)) {
  return;
}
Attachment #8633890 - Flags: feedback?(amarchesini) → feedback+
Status: NEW → ASSIGNED
Blocks: 1183666
Try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=041f5f981c57

The Gu failure was also found on other people's result, so I think the failure is not related with my patch.
Comment on attachment 8634524 [details] [diff] [review]
Don't throw exception

Hi, Baku,
Could you help me review this patch?
I think we can use this patch to fix this issue first.
After that, I will file another bug to solve the question that we can't control the audio of the attention window.
Thanks :)
Attachment #8634524 - Flags: review?(alwu) → review?(amarchesini)
Attachment #8634524 - Flags: review?(amarchesini) → review+
Note that I don't think we should fail silently here. The window.open() created iframe does not have a frameLoader in the callback by design. No other browser-api works in this case either. The client code should be fixed instead.
blocking-b2g: 2.5? → 2.5+
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][dogfood-test]
QA Whiteboard: [QAnalyst-Triage+][dogfood-test] → [QAnalyst-Triage+] [dogfood-test-blocker]
QA Whiteboard: [QAnalyst-Triage+] [dogfood-test-blocker] → [QAnalyst-Triage+] [dogfood-blocker]
try result is on the comment14.
Keywords: checkin-needed
Blocks: 1130350
Should we retitle this bug? The actual problem was a lot more severe than just breaking contacts import - this broke every single use of window.open(..., '_blank') for non-remote windows (only certified apps can open remote windows, so this is practically all of them).
Summary: [aries][Flame] import of Gmail/Outlook contacts is broken and hoses the app → [B2G] Can't open a new attention window
No longer blocks: 1184901
Whiteboard: [dogfood-blocker]
(In reply to Dietrich Ayala (:dietrich) from comment #4)
> In order to close this bug, a regression test should be added to make sure
> this cannot happen again without warning.

Looks like this never happened. Also, please give your patches more descriptive commit message in the future.
Flags: in-testsuite?
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #22)
> (In reply to Dietrich Ayala (:dietrich) from comment #4)
> > In order to close this bug, a regression test should be added to make sure
> > this cannot happen again without warning.
> 
> Looks like this never happened. Also, please give your patches more
> descriptive commit message in the future.

It's crazy to think we don't have tests in Gaia for _blank links. Agreed that we need to add this before closing this bug. I can help out here.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #22)
> (In reply to Dietrich Ayala (:dietrich) from comment #4)
> > In order to close this bug, a regression test should be added to make sure
> > this cannot happen again without warning.
> 
> Looks like this never happened. Also, please give your patches more
> descriptive commit message in the future.

Hi, Ryan,
The main crash reason is that the app window receive the error exception so that we can't open the new window. The only thing I do is not to throw the exception when we don't pass the checking of getAllowedAudioChannel().
(In reply to Michael Henretty [:mhenretty] from comment #26)
> Weird, we have a test for this. I wonder why it didn't fail. I'm looking
> into that now.
> 
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/test/marionette/
> browser_launch_window_open_test.js

Nm, I think I figure out why. This problem does not occur on b2g-desktop (probably because b2g-desktop runs in-process). In any case, this bug will not occur in our current testing infrastructure for Gaia so we cannot test the problem as reported.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S3 (24Jul)
Please check the duplicates & dependencies when verifying this bug.
I am still reproducing the issue reported by Dietrich in the description and in comment 2 of this bug with latest master build. So importing from Gmail and Outlook do not work and after pressing them the UI is broken.

Besides, Bug 1185090 - [Contacts] Cannot send MMS to email address from contact page, that was set as duplicated of this bug, is also not working.

Leaving QA team to verify it first so the bug can be reopened or it's necessary to open a new one.


Environmental variables (7/22) NOT working:
flame master (2.5.0 version)
Gecko: 8dc0177
Gaia: af4385f
Platform version: 42.0a1
This seems fixed to me on the latest master builds for Flame and Aries 2.5

The Gmail and Outlook windows are opening properly without issue. I tried multiple times in the FTE and in the Contacts app and everything functioned as expected. 

Device: Flame 2.5 (Full Flash)(KK)(319mb)
BuildID: 20150722010204
Gaia: 84c3bf622e211046d905803b34de5d331761f22d
Gecko: 8e5c888d0d89
Gonk: 41d3e221039d1c4486fc13ff26793a7a39226423
Version: 42.0a1 (2.5) 
Firmware Version: v18D-4
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Device: Aries 2.5
BuildID: 20150619225606
Gaia: 4c06ed88ddccaba8dc941e5006bd2a9e57306f07
Gecko: 7c1a6b1151a1539186b950a144387e2d7f378d1b
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 41.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

I am not sure why Maria is getting different results than me. I do see that bug 1185090 is still occurring though so I will reopen that issue and close this one out.
Status: RESOLVED → VERIFIED
Thanks a lo Kevin!
I suspect that there could be a problem with the build generated here in Telefonica. Tomorrow I will retest it.
I tested the bug on 7/21 when the gecko patch attached in this bug landed in mozilla-central (or at least when the status of the bug changed to Resolved). With only this patch the issue is NOT resolved, or at least I continue reproducing it.

Last night, Bug 1181982 - [B2G] Manage the sound of the attention window (7/23), landed in Gaia and right now (after including that commit) the issue is resolved (the gmail/outlook importing).

No idea why Kevin was able to see the issue solved yesterday. Anyway, now it's fixed :) 

Environmental variables (7/23) NOT working:
flame master (2.5.0 version)
Gecko: 8cd8e1b
Gaia: 2ae9a4c
Platform version: 42.0a1
My guess is that it depends on her builds's repo.  Which repo is this pull from?  I can't seem to find it.
Flags: needinfo?(oteo)
I think we are pulling from the same repos than you. 

Perhaps Alastor can confirm if bug 1181982 was necessary to fix the gmail/outlook importing issue...
Flags: needinfo?(oteo) → needinfo?(alwu)
Hi, all,
As I know, the issue (can't open the attention window) could be solved by this bug.

The reason of opening the new window fail is that if any error exception be thrown from the app_window.js, the window opening process would be stopped. If you can still reproduce the problem, that might be another different problem :)
Flags: needinfo?(alwu)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: