1058387 - [NFC] System browser could not share via NFC anymore once app browser has ever been launched

Status: VERIFIED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Alison Shiue]

Device    Flame
Gaia      1934a2297ffc0d90424cd9cd3294c4a8c74a7333
Gecko     https://hg.mozilla.org/mozilla-central/rev/18901d4f3edd
BuildID   20140825160203
Version   34.0a1

*Prerequisite:
(1) 2 phones(Device A and B) enable NFC

STR:
1. Device A launch app browser and open website A
2. Device A open website B via rocket bar (running at foreground)
3. Tap two phones together, and Device A swipe shrinking UI
4. Device A kill app browser (website B is running at foreground)
5. Tap two phones together, and Device A swipe shrinking UI

Expect result:
1. In step 3, Device B should reveice website B
2. In step 5, Device B should reveice website B

Actual result:
1. In step 3, Device B should reveice website A
2. In step 5, Device B receive nothing

Comment 1

[Alison Shiue]

[Blocking Requested - why for this release]:

Comment 2

[howie [:howie]]

Triage: blocker

Comment 3

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Yoshi,

I've tried printing some message as https://github.com/cctuan/gaia/commit/feabefc087a4bdc09df418954bf04dadfc5af529 when repro STR 1 to 3.

The manifestURL gaia put for nfcdom.notifyUserAcceptedP2P is apps://system.gaiamobile.org/manifest.webapp. However, mozNFC trigger BROWSER app's onpeerready, instead of system's.

Is it possible that system's onpeerready(in system/js/nfc_handler.js) is override by other app? If that so, is that what we expect?

Comment 4

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

I'll check if this is Gecko bug.

Comment 5

[Garner Lee]

Hi Fabrice, 

I think we had this dicussion in the past about multiple "window" support in a single app.

NFC registers peers once only by "appId". Browser v1, and Browser v2 actually has the same AppID (same system process), can we disambiguate them (different ManifestURL, different instance of the same app)?

Comment 6

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

The appIds are different, however they run on the same process.

And App Browser is going to be removed, after discussed with George, System browser should listen to nfc-ndef-discovered MozActivity, meanwhile he will discuss with owner of App Browser if removing the nfc-ndef-discovered mozAcvitiy is still neccesary.

Comment 7

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master

Hi Alive and Ben,
as comment 6 has mentioned, browser app is going to be removed, I think we should launch system browser instead of browser app if get a nfc activity. Does that make sense to you?

Comment 8

[Ben Francis [:benfrancis]]

Comment on attachment 8481048 [details] [review]
PR to master

This is a weird use of Web Activities, but yes that will be necessary if you want to load a URL in the system browser via an NFC web activity.

IMHO there should be an NFC app which fires a "view URL" web activity, but I've said this many times before.

Comment 9

[Krzysztof Mioduszewski[:tauzen]]

(In reply to Ben Francis [:benfrancis] from comment #8)
> Comment on attachment 8481048 [details] [review]
> PR to master
> 
> This is a weird use of Web Activities, but yes that will be necessary if you
> want to load a URL in the system browser via an NFC web activity.
> 
> IMHO there should be an NFC app which fires a "view URL" web activity, but
> I've said this many times before.

NfcManager fires the NFC NDEF activities. We can change the name property to view in here [1] and the additional entry in manifest for system browser should not be needed. 

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/nfc_manager.js#L465

Comment 10

[Ben Francis [:benfrancis]]

(In reply to Krzysztof Mioduszewski[:kmioduszewski][:tauzen] from comment #9)
> NfcManager fires the NFC NDEF activities. We can change the name property to
> view in here [1] and the additional entry in manifest for system browser
> should not be needed.

That sounds much better, if it would work.

Comment 11

[Krzysztof Mioduszewski[:tauzen]]

Done a very quick test and it seems to work. I've added |options.name = 'view'| above the line I linked in comment 9. System browser is launched and sharing works ok. We're already doing similar handling for mail and contact app, not sure why it was implemented this way for the browser.

I think this is a good solution, it will require also updating the smartposter handling and unit tests. George feel free to ping me on IRC if you need some help.

Comment 12

[Garner Lee]

The main issue with firing the activities as non-NDEF messages is simply that you get an ever growing list of types that NFC Manager needs to be aware of.

For limited special cases where it is a well known type for the FirefoxOS platform, I think it's fine, so long as the app gets an extra field to indicate the activity was from an NFC source. Context might matter for UX or security reasons.

Comment 13

[Krzysztof Mioduszewski[:tauzen]]

(In reply to Garner Lee from comment #12)
> The main issue with firing the activities as non-NDEF messages is simply
> that you get an ever growing list of types that NFC Manager needs to be
> aware of.

There is bug 1016554 for unifying how apps handle URI/URL related activities which will probably address the growing list of types.

> For limited special cases where it is a well known type for the FirefoxOS
> platform, I think it's fine, so long as the app gets an extra field to
> indicate the activity was from an NFC source. Context might matter for UX or
> security reasons.

Yesterday I've raised bug 1060403 to add the extra field. So we're all good here ;).

Comment 14

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8481048 [details] [review]
PR to master

Hi Alive,
this patch is based on comment 11, and I tested it, it works file.
Could you review this one?
Thanks.

Comment 15

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

Comment on attachment 8481048 [details] [review]
PR to master

Much better

Comment 16

[(Inactive after June) George Duan [:gduan] [:喬智]]

Thanks,
master: https://github.com/mozilla-b2g/gaia/pull/23449

Comment 17

[Alison Shiue]

Verified on
Gaia      fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/fb5e796da813
BuildID   20140903000204
Version   34.0a2

No longer use app browser when NFC sharing.

Comment 18

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.1

Comment 19

[Shally Li（leave from Mozilla）]

This bug has been verified successfully on Flame v2.1

Flame 2.1 build:
Gaia-Rev        afdfa629be209dd53a1b7b6d6c95eab7077ffcd9
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/dc3018cbdbe6
Build-ID        20141123001201
Version         34.0