1129841 - setNFCFocus(false) won't be called when the app goes to the background

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

+++ This bug was initially created as a clone of Bug #1125023 +++

STR:
1. Enable NFC
2. Launch NFC API Tests -> NFC Read Tag
3. Press Home key to Homescreen
4. Tab an NFC Tag containing URL.

Expected Result:
System app launches Browser showing the URL contained in the tag.

Actual Result:
Nothing happened, launch the app NFC API Tests again, found the content of result is shown in the app.

When the app goes to the background, I observed that setNFCFocus(false) won't be called on the Browser Element of the the NFC API test app, which should be wrong since the app is in foreground now.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I found 2.2 also has this problem

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Yoshi Huang[:allstars.chh] from comment #0)
> +++ This bug was initially created as a clone of Bug #1125023 +++
> 
> STR:
> 1. Enable NFC
> 2. Launch NFC API Tests -> NFC Read Tag
> 3. Press Home key to Homescreen
> 4. Tab an NFC Tag containing URL.
> 
> Expected Result:
> System app launches Browser showing the URL contained in the tag.
> 
> Actual Result:
> Nothing happened, launch the app NFC API Tests again, found the content of
> result is shown in the app.
> 
> When the app goes to the background, I observed that setNFCFocus(false)
> won't be called on the Browser Element of the the NFC API test app, which
> should be wrong since the app is in foreground now.
                                     ^^^^^^^^^^^^^^^^
I mean 'since the app is in the *background* now'

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

ni? Alive for comments.

Comment 4

[howie [:howie]]

Triage: blocking

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Is this really a bug? Don't we setNFCFocus(true) to homescreen app while pressing home button? Why is it necessary to setNFCFocus(false) to closing app?

Comment 6

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

I didn't see setNFCFocus(true) for Homescreen when normal app goes to background.

Comment 7

[GitHub Autolander Bot]

Attachment: [PullReq] alivedise:bugzilla/1129841_master/setNFCFocus-on-setVisible to mozilla-b2g:master

Comment 8

[Dimi Lee [:dimi]]

Hi Alive,
One question, now gecko make an assumption that when GAIA change focus app to another app(ex, App A to App B),
GAIA will always call App A's setNFCFocus(false) before calling App B's setNFCFocus(true).

Can we make this assumption ? or it is possible that GAIA will call
1.A.setNFCFocus(true)
2.B.setNFCFocus(true)
without calling A.setNFCFocus(false)

Comment 9

[Etienne Segonzac (:etienne)]

Comment on attachment 8562135 [details] [review]
[PullReq] alivedise:bugzilla/1129841_master/setNFCFocus-on-setVisible to mozilla-b2g:master

lgtm

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Dimi Lee[:dimi][:dlee] from comment #8)
> Hi Alive,
> One question, now gecko make an assumption that when GAIA change focus app
> to another app(ex, App A to App B),
> GAIA will always call App A's setNFCFocus(false) before calling App B's
> setNFCFocus(true).
> 
> Can we make this assumption ? or it is possible that GAIA will call
> 1.A.setNFCFocus(true)
> 2.B.setNFCFocus(true)
> without calling A.setNFCFocus(false)

No, we didn't do this I think. What we do is just trying to tell you when to unfocus the current focused app or focus a new one when there is.
setNFCFocus(true) to A then setNFCFocus(true) to B is possible with setNFCFocus(false) to A happens before/after or even not called.

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo@Paris~2/17 [:alive][NEEDINFO!] from comment #10)
> (In reply to Dimi Lee[:dimi][:dlee] from comment #8)
> > Hi Alive,
> > One question, now gecko make an assumption that when GAIA change focus app
> > to another app(ex, App A to App B),
> > GAIA will always call App A's setNFCFocus(false) before calling App B's
> > setNFCFocus(true).
> > 
> > Can we make this assumption ? or it is possible that GAIA will call
> > 1.A.setNFCFocus(true)
> > 2.B.setNFCFocus(true)
> > without calling A.setNFCFocus(false)
> 
> No, we didn't do this I think. What we do is just trying to tell you when to
> unfocus the current focused app or focus a new one when there is.
> setNFCFocus(true) to A then setNFCFocus(true) to B is possible with
> setNFCFocus(false) to A happens before/after or even not called.

More accurately:
* When an app is setNFCFocus(true): it should be the top most frame at the moment; for gecko that means it should keep this one as focused no matter the previously focused is null or another one.
* When an app is setNFCFocus(false): it should not get focus at the moment; for gecko that means it should forget it as the focused frame.

Comment 12

[Dimi Lee [:dimi]]

(In reply to Alive Kuo@Paris~2/17 [:alive][NEEDINFO!] from comment #11)
> (In reply to Alive Kuo@Paris~2/17 [:alive][NEEDINFO!] from comment #10)
> > (In reply to Dimi Lee[:dimi][:dlee] from comment #8)
> > > Hi Alive,
> > > One question, now gecko make an assumption that when GAIA change focus app
> > > to another app(ex, App A to App B),
> > > GAIA will always call App A's setNFCFocus(false) before calling App B's
> > > setNFCFocus(true).
> > > 
> > > Can we make this assumption ? or it is possible that GAIA will call
> > > 1.A.setNFCFocus(true)
> > > 2.B.setNFCFocus(true)
> > > without calling A.setNFCFocus(false)
> > 
> > No, we didn't do this I think. What we do is just trying to tell you when to
> > unfocus the current focused app or focus a new one when there is.
> > setNFCFocus(true) to A then setNFCFocus(true) to B is possible with
> > setNFCFocus(false) to A happens before/after or even not called.
> 
> More accurately:
> * When an app is setNFCFocus(true): it should be the top most frame at the
> moment; for gecko that means it should keep this one as focused no matter
> the previously focused is null or another one.
> * When an app is setNFCFocus(false): it should not get focus at the moment;
> for gecko that means it should forget it as the focused frame.

Thanks for the information, I have created bug 1131964 for that.

Comment 13

[(inactive after 6/18) Alive Kuo [:alive]]

Try is green; waiting your guys' confirm to land this.

Comment 14

[Dimi Lee [:dimi]]

Hi Alive,
When App A launched, and press home key. It will call setNFCFocus(false) to App A, but it won't call setNFCFocus(true) to homescreen. Is this expected behavior ?

Comment 15

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8562135 [details] [review]
[PullReq] alivedise:bugzilla/1129841_master/setNFCFocus-on-setVisible to mozilla-b2g:master

Although this PR fixes this bug, but Bug 1125023 will be back again.

Also I didn't see any log for calling setNFCFocus(true) on Homescreen app.

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8562135 [details] [review]
[PullReq] alivedise:bugzilla/1129841_master/setNFCFocus-on-setVisible to mozilla-b2g:master

Let's see if this works

Comment 17

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8562135 [details] [review]
[PullReq] alivedise:bugzilla/1129841_master/setNFCFocus-on-setVisible to mozilla-b2g:master

Happy Chinese New Year \O/

Comment 18

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8562135 [details] [review]
[PullReq] alivedise:bugzilla/1129841_master/setNFCFocus-on-setVisible to mozilla-b2g:master

Re-request review because I changed the logic to prevent regression bug 1125023 - it's because setVisible(false) is called when ShrinkingUI is started. I think this way makes more sense because shouldFocus covers the case of this one and we will not focus something that is not loaded yet according to current app transition controller behavior.

Comment 19

[Etienne Segonzac (:etienne)]

Comment on attachment 8562135 [details] [review]
[PullReq] alivedise:bugzilla/1129841_master/setNFCFocus-on-setVisible to mozilla-b2g:master

(In reply to Alive Kuo@Paris~2/17 [:alive][NEEDINFO!] from comment #18)
> I think this way makes more sense because shouldFocus covers the
> case of this one and we will not focus something that is not loaded yet
> according to current app transition controller behavior.

yep :)

Comment 20

[(inactive after 6/18) Alive Kuo [:alive]]

https://github.com/mozilla-b2g/gaia/commit/c357b8877d96e8009f1f98c84704dc0462ce42ce

Comment 21

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8562135 [details] [review]
[PullReq] alivedise:bugzilla/1129841_master/setNFCFocus-on-setVisible to mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): regression of bug 1125023
[User impact] if declined: NFC cannot be used
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): riskless
[String changes made]: No

Comment 22

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/857cab9fd5e173928c6ab8759577f7fb8e64996b