All users were logged out of Bugzilla on October 13th, 2018

[Gaia] Support sharing of URLs

RESOLVED FIXED in 1.3 Sprint 5 - 11/22

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: frlee, Assigned: s.x.veerapandian)

Tracking

({productwanted})

unspecified
1.3 Sprint 5 - 11/22
x86
Mac OS X
productwanted
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:-)

Details

(Whiteboard: [systemsfe])

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

5 years ago
for NFC, Browser application needs to be able to create NDEF message which contains URL info. so that users can share URL via NFC.
(Reporter)

Updated

5 years ago
Depends on: 894678
(Reporter)

Comment 1

5 years ago
The user can - from the browser - share the URL of the page that
the browser is currently pointing to. The URL can be shared with another
NFC enabled device by tapping the devices together.
a) The user selects “Share Web Page” from the browser menu.
b) The user taps the device with another NFC enabled device.
c) On the receiving mobile, the device will get a prompt saying “Another device wants to share an URL with you – Accept/Reject” or something similar.
d) Upon accepting, the receiving mobile opens up the browser and goes to the URL.
Summary: [Gaia] Browser application: create NDEF message contains URL info → [Gaia] Support sharing of URLs

Updated

5 years ago
Blocks: 894678
No longer depends on: 894678

Comment 2

5 years ago
Hi Ben, are you available to help this bug?
Flags: needinfo?(bfrancis)
Hi Ken,

The browser app already has a share URL feature in the master branch, it fires a "share URL" web activity which any app can consume.

From the user requirements described here it sounds like the best way to implement this would be to create an NFC sharing app which can consume a "share URL" web activity from any app, including the browser, and has access to the NFC API to generate the NFC message.

If you think there are any new user requirements that need to be implemented in the browser app, the browser team (https://wiki.mozilla.org/FirefoxOS/SprintStatus#Browser) can assess those and prioritise them in the product backlog during the next sprint planning meeting along with all other feature requests.
Flags: needinfo?(bfrancis)

Comment 4

5 years ago
Candice, can you find someone from Browser team to own this bug? Thank you.
blocking-b2g: --- → 1.3+
Flags: needinfo?(cserran)
Whiteboard: [1.3:p1]
Whiteboard: [1.3:p1] → [1.3:p1][systemsfe]

Comment 5

5 years ago
Created attachment 820728 [details] [diff] [review]
Patch (v1) Browser App Example changes to support NFC URI NDEF messages.

This is an example of how to modify an application to get a nfc-ndef-discovered message. It basically just opens the web page any other incoming activity in this particular instance.

Comment 6

5 years ago
Answering email comments:
> Why "nfc-ndef-discovered" instead of "view".

Unlike "view"
"nfc-ndef-discovered' will also return the entire NDEF Record, see WebIDL here: https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=674741&attachment=823735

A few definitions:
NFC stands for Near Field Communications.
NDEF stands for: NFC Data Exchange Format.

So, if the application has full knowledge of what an NDEF Record is, or even has a desire for very special handling for NFC tags, the distinction between "view and nfc-ndef-discovered is potentially important, as the NDEF message is delivered as a array of one or more NDEFRecords.

Sending NDEF messages:
In the case of Browser NFC Peer to Peer communications, the next API to be exposed is the "onpeerfound" and "onpeerlost". Next patch, after NFC DOM landing.

Updated

5 years ago
Blocks: 933640

Comment 7

5 years ago
Ben, are you the one who could be the assignee on this bug? If you're not, I am sorry and please feel free to clear your name on the assignee. Thank you.
Assignee: nobody → bfrancis
We don't have time for this in the 1.3 time frame. We also need a device for this.
I can look into this further if it gets into the Systems FE backlog and planned for a sprint.
Assignee: bfrancis → nobody

Comment 10

5 years ago
It seems better to finish this bug before sprint5. If you think it isn't reasonable, please update target milestone.

Updated

5 years ago
Target Milestone: --- → 1.3 Sprint 5 - 11/22

Updated

5 years ago
Whiteboard: [1.3:p1][systemsfe] → [systemsfe]
NFC is not a committed feature for 1.3, so this should not block the release.
blocking-b2g: 1.3+ → 1.3?

Updated

5 years ago
Component: NFC → Gaia::Browser
Moving this out of NFC into browser as the code changes here would happen in the browser app, not the NFC codebase.
Gregor & I discussed this offline - we can't block on this at this point, as this is not a committed feature for 1.3. I do wonder if bug 920882 might have fixed this issue, so I'm putting needinfo on Greg to see what he thinks.
blocking-b2g: 1.3? → -
Flags: needinfo?(gweng)
I believe that Shrinking UI doesn't and shouldn't directly communicate with any applications, according to the discussions before. Shrinking UI should only accept some limited events from NFC manager. So the path would be

    NFC manager -> (peer found)
    Browser -> (decide to send or not)
    sending|not sending -> (call sending method)
    NFC manager -> (notify the shrinking ui)
    Shrinking UI -> (notify that user has tapped and swiped)
    NFC manager -> (do the sending thing)
    ...
Flags: needinfo?(gweng)
(If I'm wrong please correct me)

Comment 16

5 years ago
Ben, are you the best one to handle this bug? Thanks.
Assignee: nobody → bfrancis
Kevin, I have already un-assigned myself once. Please do not assign bugs to me.

Peter for prioritisation.
Flags: needinfo?(pdolanjski)
Keywords: productwanted
Assignee: bfrancis → nobody

Comment 18

5 years ago
Peter, partner would like to have a NFC demo in MWC. I wonder if you could find someone to handle this bug.
Thanks very much.
(Assignee)

Comment 19

5 years ago
Created attachment 8355163 [details] [diff] [review]
NfcUri.patch
Attachment #8355163 - Flags: review?(bfrancis)
(Assignee)

Comment 20

5 years ago
Created attachment 8355164 [details] [diff] [review]
NfcUri.patch

Please find the NFC URI implementation patch. 

Implementation Details:
1.	nfcURIPeerHandler() , get the URL from currently opened TAB in the Browser.
2.	Create NDEF record based on the {Url, tnf,id and rtd} inputs.
3.	Transmit the NDEF record via NFC peer connectivity.
4.	handleVisibilityChange(), Provides the Browser Hidden and Visible status. Based on that nfcURIPeerHandler can be added and removed.
5.	createUriNdefRecord() , creates the NDEF records, It Looks up the URL record type and create the Payload data and Payload Identifier.
6.	sendNdefRecords(), Create the Nfc Peer connectivity and Transmit the NDEF records.

OnGoing:
7.	Currently, I could not able to locate where exactly I have remove the handler (mozNfc.onpeerready = null).
a.	Browser visibility handled in handleVisibilityChange(). But during shrinking URI inside Browser,  handleVisibilityChange() has invoked, the value of document.hidden becomes true.
Attachment #8355164 - Flags: review?(bfrancis)

Comment 21

5 years ago
Comment on attachment 8355164 [details] [diff] [review]
NfcUri.patch

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

::: apps/browser/js/browser.js
@@ +602,5 @@
> +    records.push(record);
> +    res = NfcURI.sendNdefRecords(records,event);
> +
> +    if (!res)
> +      debug('URL transfer failed');;

It should have only one ";", right?
(Assignee)

Comment 22

5 years ago
Created attachment 8355536 [details]
0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch

Here with I attached the Patch for review.
Also Find the PR https://github.com/mozilla-b2g/gaia/pull/15012
Attachment #8355163 - Attachment is obsolete: true
Attachment #8355164 - Attachment is obsolete: true
Attachment #8355163 - Flags: review?(bfrancis)
Attachment #8355164 - Flags: review?(bfrancis)
Attachment #8355536 - Flags: review?(khu)
Attachment #8355536 - Flags: review?(bfrancis)

Comment 23

5 years ago
Comment on attachment 8355536 [details]
0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch

Review of attachment 8355536 [details]:
-----------------------------------------------------------------

::: apps/communications/contacts/js/contacts.js
@@ +264,5 @@
> +    var contact = {};
> +    var str = '';
> +    var count = 0;
> +    var tnf_mime_media = 0x02;
> +    var VCARD_VERSION = '2.1'

Should we add a ';' at the end of this line? Also, we should use version 2.1 here? There has a shared 'contact2vcard.js' file that defines 4.0 as the VCARD version. Maybe we can use that one? Or, we are using 2.1 for other reasons?

@@ +288,5 @@
> +
> +    LazyLoader.load('/shared/js/contact2vcard.js', function() {
> +    ContactToVcard([contact], VCARD_VERSION, function append(vcards, nCards) {
> +    str += vcards;
> +    count += nCards;

I don't understand what's the purpose for the local variable 'count'.

@@ +293,5 @@
> +    }, function success() {
> +        var data;
> +
> +        data = contains(str)
> +        if (data){

Looks like 'data' is used to check the return value from contains() only. Would it be simpler to write it as: 

if(contains(str)){
  payload = str;
}
(Assignee)

Comment 24

5 years ago
Created attachment 8356034 [details]
15012[2]

Please find the NFC URI implementation:

Implementation Details:
1. nfcURIPeerHandler() is the peer handler function. It gets the URL from currently opened TAB in the Browser.
2. To Create NDEF record based on the {UrlData, tnf,id and rtd,Id} inputs.
3. To transmit the NDEF record via NFC peer connectivity.
4. handleVisibilityChange(), Provides the Browser Hidden and Visibility status. Based on that nfcURIPeerHandler can be added and removed.
5.createUriNdefRecord() , creates the NDEF records, It Looks up the URL record type and create the Payload data and Payload Identifier.
6. sendNdefRecords(), Create the Nfc Peer connectivity and Transmit the NDEF records.
7. Browser Exit Handle in Normal and Shrink mode, using the ‘NFC-Manager-Tech-Discovered’ message handler. If ‘NFC-Manager’ is enabled and ‘Browser Status’ is hidden. I am considering the NFC Paring is in progress, other-wise, Browser is Exit by User (mozNfc.onpeerready = null).
Attachment #8356034 - Flags: review?(kchang)

Comment 25

5 years ago
Comment on attachment 8356034 [details]
15012[2]

I am not a proper reviewer for this patch...:-(.
Attachment #8356034 - Flags: review?(kchang) → review?(bfrancis)
Comment on attachment 8355536 [details]
0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch

Is this patch attached to the wrong bug? It doesn't seem related to URL sharing in the browser app.

You would need someone from the communications team to review this I'm afraid.

Looking at the other patch.
Attachment #8355536 - Flags: review?(khu)
Attachment #8355536 - Flags: review?(bfrancis)
Comment on attachment 8356034 [details]
15012[2]

Thanks for the patch!

See my comments and questions on the pull request https://github.com/mozilla-b2g/gaia/pull/15012

Please note that while we can land this in the browser app for demo purposes, this is unlikely to ever ship in a release because the browser app is being replace by the new system browser in the system app.
Attachment #8356034 - Flags: review?(bfrancis) → review-

Comment 28

5 years ago
(In reply to Ben Francis [:benfrancis] from comment #26)
> Comment on attachment 8355536 [details]
> 0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch
> 
> Is this patch attached to the wrong bug? It doesn't seem related to URL
> sharing in the browser app.
> 
> You would need someone from the communications team to review this I'm
> afraid.
> 
> Looking at the other patch.

Yes, I believe that this one was attached to the wrong bug.

Comment 29

5 years ago
Comment on attachment 8355536 [details]
0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch

Obsolete 0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch to avoid confusion.
Attachment #8355536 - Attachment is obsolete: true
Attachment #8355536 - Attachment is patch: false
(Assignee)

Comment 30

5 years ago
Created attachment 8357136 [details]
15106[1]

1. The constant are URI Record Type Definition. These are mapped to URI string prefixes. URI Identifier code and URL data transfer via NFC
2. All Handler functions are changed from Browser to NFC and Code kept in the nfc.js 
3. String utility moved into utilities.js 
4. gjslint build error is verified 
5. Variables are camelCase
Attachment #8356034 - Attachment is obsolete: true
Attachment #8357136 - Flags: review?(bfrancis)
Comment on attachment 8357136 [details]
15106[1]

Great, thanks for the contribution!
Attachment #8357136 - Flags: review?(bfrancis) → review+
Please fix the remaining lint errors so we can ensure Travis is green before merging

https://github.com/mozilla-b2g/gaia/pull/15106

----- FILE : /home/travis/build/mozilla-b2g/gaia/apps/browser/js/browser.js -----

Line 600, E:0005: Illegal tab in whitespace before "}"

Line 602, E:0110: Line too long (97 characters).

----- FILE : /home/travis/build/mozilla-b2g/gaia/apps/browser/js/nfc.js -----

Line 64, E:0110: Line too long (89 characters).

Line 120, E:0001: Extra space at end of line

Line 142, E:0110: Line too long (81 characters).

Line 151, E:0121: Illegal comma at end of object literal

----- FILE : /home/travis/build/mozilla-b2g/gaia/apps/browser/js/utilities.js -----

Line 125, E:0121: Illegal comma at end of object literal

Found 7 errors, including 0 new errors, in 3 files (1477 files OK).
Flags: needinfo?(s.x.veerapandian)
(Assignee)

Comment 33

5 years ago
Created attachment 8357725 [details]
15146[1]

I Fixed the Errors. Travis CI build passed
Attachment #8357725 - Flags: review?(bfrancis)
Landed in https://github.com/mozilla-b2g/gaia/commit/eff2c8c11f7c3b22c6e5ac5832a3a169fecf4841
Travis: https://travis-ci.org/mozilla-b2g/gaia/builds/16643879
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Assignee: nobody → s.x.veerapandian
(Assignee)

Updated

5 years ago
Attachment #8357725 - Flags: review?(bfrancis)
(Assignee)

Updated

5 years ago
Flags: needinfo?(s.x.veerapandian)
Flags: needinfo?(pdolanjski)
Flags: needinfo?(cserran)
You need to log in before you can comment on or make changes to this bug.