Closed
Bug 903250
Opened 11 years ago
Closed 11 years ago
[Gaia] Support sharing of URLs
Categories
(Firefox OS Graveyard :: Gaia::Browser, defect)
Tracking
(blocking-b2g:-)
People
(Reporter: frlee, Assigned: s.x.veerapandian)
References
Details
(Keywords: productwanted, Whiteboard: [systemsfe])
Attachments
(3 files, 4 obsolete files)
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 | ||
Comment 1•11 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•11 years ago
|
Comment 3•11 years ago
|
||
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•11 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]
Updated•11 years ago
|
Whiteboard: [1.3:p1] → [1.3:p1][systemsfe]
Comment 5•11 years ago
|
||
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•11 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.
Comment 7•11 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
Comment 8•11 years ago
|
||
We don't have time for this in the 1.3 time frame. We also need a device for this.
Comment 9•11 years ago
|
||
I can look into this further if it gets into the Systems FE backlog and planned for a sprint.
Assignee: bfrancis → nobody
Comment 10•11 years ago
|
||
It seems better to finish this bug before sprint5. If you think it isn't reasonable, please update target milestone.
Updated•11 years ago
|
Target Milestone: --- → 1.3 Sprint 5 - 11/22
Updated•11 years ago
|
Whiteboard: [1.3:p1][systemsfe] → [systemsfe]
Comment 11•11 years ago
|
||
NFC is not a committed feature for 1.3, so this should not block the release.
blocking-b2g: 1.3+ → 1.3?
Updated•11 years ago
|
Component: NFC → Gaia::Browser
Comment 12•11 years ago
|
||
Moving this out of NFC into browser as the code changes here would happen in the browser app, not the NFC codebase.
Comment 13•11 years ago
|
||
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)
Comment 14•11 years ago
|
||
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)
Comment 15•11 years ago
|
||
(If I'm wrong please correct me)
Comment 16•11 years ago
|
||
Ben, are you the best one to handle this bug? Thanks.
Assignee: nobody → bfrancis
Comment 17•11 years ago
|
||
Kevin, I have already un-assigned myself once. Please do not assign bugs to me.
Peter for prioritisation.
Flags: needinfo?(pdolanjski)
Keywords: productwanted
Updated•11 years ago
|
Assignee: bfrancis → nobody
Comment 18•11 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•11 years ago
|
||
Attachment #8355163 -
Flags: review?(bfrancis)
Assignee | ||
Comment 20•11 years ago
|
||
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•11 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•11 years ago
|
||
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•11 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•11 years ago
|
||
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•11 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 26•11 years ago
|
||
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 27•11 years ago
|
||
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•11 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•11 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•11 years ago
|
||
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 31•11 years ago
|
||
Comment on attachment 8357136 [details]
15106[1]
Great, thanks for the contribution!
Attachment #8357136 -
Flags: review?(bfrancis) → review+
Comment 32•11 years ago
|
||
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•11 years ago
|
||
I Fixed the Errors. Travis CI build passed
Attachment #8357725 -
Flags: review?(bfrancis)
Comment 34•11 years ago
|
||
Landed in https://github.com/mozilla-b2g/gaia/commit/eff2c8c11f7c3b22c6e5ac5832a3a169fecf4841
Travis: https://travis-ci.org/mozilla-b2g/gaia/builds/16643879
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee: nobody → s.x.veerapandian
Attachment #8357725 -
Flags: review?(bfrancis)
Updated•11 years ago
|
Flags: needinfo?(pdolanjski)
Updated•11 years ago
|
Flags: needinfo?(cserran)
You need to log in
before you can comment on or make changes to this bug.
Description
•