1183301 - [B2G] Can't open a new attention window

Status: VERIFIED FIXED

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

Description

[Dietrich Ayala (:dietrich)]

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.

Comment 1

[Dietrich Ayala (:dietrich)]

Build is from today: https://tools.taskcluster.net/index/artifacts/#gecko.v1.mozilla-central.latest.linux.aries-dogfood/gecko.v1.mozilla-central.latest.linux.aries-dogfood.debug

Comment 2

[Maria Angeles Oteo (:oteo)]

[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

Comment 3

[Maria Angeles Oteo (:oteo)]

Kevin, does this bug block the smoketest?

Comment 4

[Dietrich Ayala (:dietrich)]

In order to close this bug, a regression test should be added to make sure this cannot happen again without warning.

Comment 5

[KTucker [:KTucker][Inactive 3/4/2016]]

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.

Comment 6

[KTucker [:KTucker][Inactive 3/4/2016]]

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.

Comment 7

[Peter Bylenga [:PBylenga]]

NI on author and reviewer for commits in bug 1113086, looks like it caused this severe issue due to the found window.

Comment 8

[Oliver Nelson [:oliverthor]]

Attachment: logcat_20150714_1402.txt

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

Comment 9

[Alastor Wu [:alwu]]

Attachment: Not throw error

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!

Comment 10

[Alastor Wu [:alwu]]

Need another better way.

Comment 11

[Andrea Marchesini [:baku]]

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;
}

Comment 13

[Alastor Wu [:alwu]]

Attachment: 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 :)

Comment 14

[Alastor Wu [:alwu]]

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 15

[Alastor Wu [:alwu]]

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 :)

Comment 17

[Kan-Ru Chen [:kanru] (UTC+9)]

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.

Comment 18

[Alastor Wu [:alwu]]

try result is on the comment14.

Comment 19

[Chris Lord [:cwiiis]]

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).

Comment 22

[Ryan VanderMeulen [:RyanVM]]

(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.

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/c1bae5ba297f

Comment 24

[Michael Henretty [:mikehenrty][:mhenretty]]

(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.

Comment 25

[Alastor Wu [:alwu]]

(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().

Comment 26

[Michael Henretty [:mikehenrty][:mhenretty]]

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

Comment 27

[Michael Henretty [:mikehenrty][:mhenretty]]

(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.

Comment 29

[Carsten Book [:Tomcat]]

http://hg.mozilla.org/mozilla-central/rev/c1bae5ba297f note that this also landed on m-c

Comment 30

[Peter Bylenga [:PBylenga]]

Please check the duplicates & dependencies when verifying this bug.

Comment 31

[Maria Angeles Oteo (:oteo)]

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

Comment 32

[KTucker [:KTucker][Inactive 3/4/2016]]

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.

Comment 33

[Maria Angeles Oteo (:oteo)]

Thanks a lo Kevin!
I suspect that there could be a problem with the build generated here in Telefonica. Tomorrow I will retest it.

Comment 34

[Maria Angeles Oteo (:oteo)]

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

Comment 35

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

My guess is that it depends on her builds's repo.  Which repo is this pull from?  I can't seem to find it.

Comment 36

[Maria Angeles Oteo (: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...

Comment 37

[Alastor Wu [: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 :)