Closed Bug 853716 Opened 11 years ago Closed 11 years ago

[Buri][Youtube]Share the Youtube video from SMS/EMAIL/FACEBOOK/TWITTER/GOOGLE+,the youtube should use native app,not page

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:tef+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

RESOLVED FIXED
B2G C4 (2jan on)
blocking-b2g tef+
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: sync-1, Assigned: fabrice)

References

Details

(Whiteboard: [apps watch list1], QARegressExclude)

Attachments

(4 files, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #419624 +++
 
 DEFECT DESCRIPTION:
 Share the Youtube video from SMS/EMAIL/FACEBOOK/TWITTER/GOOGLE+,the youtube should  use native app,not page
 
 REPRODUCING PROCEDURES:
 1.Into Youtube apk,play the any youtube
 2.Share this video link from SMS/EMAIL/FACEBOOK/TWITTER/GOOGLE+...
 3.Then share from sms and email,it use native app
 4.Then share from Facebook/Twitter/Google+ other app,it user network page---ko
 
 EXPECTED BEHAVIOUR:
 For ko,when share some video link from Facebook/Twitter/Google+,the youtube should use native app
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 Case ID B2G-104868
 TOOLS AND PLATFORMS USED:
 
 USER IMPACT:
 
 REPRODUCING RATE:
 
 For FT PR, Please list reference mobile's behavior:
 
 AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.19.037
 Firefox os  v1.0.1
 Mozilla build ID: 20130310070203.
 
 ++++++++++ end of initial bug #419624 description ++++++++++
 
 
 
 CONTACT INFO (Name,Phone number):
 
 DEFECT DESCRIPTION:
 
 REPRODUCING PROCEDURES:
 
 EXPECTED BEHAVIOUR:
 
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 
 TOOLS AND PLATFORMS USED:
 
 USER IMPACT:
 
 REPRODUCING RATE:
 
 For FT PR, Please list reference mobile's behavior:
blocking-b2g: --- → tef?
Priority: P2 → P1
Sorry, it is not clear to me what is exactly this bug about. Could you please provide more details about this bug?
Flags: needinfo?(sync-1)
potentially a bug on youtube website?
I know Dale (currently on PTO) and Fabrice have worked on YouTube-related things in the past.  Is it possible that we're not getting redirected properly?
Sorry, not on PTO anymore, forgot to update my bugzilla

There is no code to redirect youtube urls to the native application, the youtube website uses a special protocol which we handle to open the Video app. If we want to open the video app automatically for any links to youtube pages thats a new feature afaik (and I dont know the implications enough to determine whether its a good idea or not)
actually disregard that, I thought it was asking about direct links to video on youtube, not even sure thats possible, if you share a youtube link over sms etc it will open in the browser, the browser will then open the video app.

So yeh need more information to know what the bug is here
I agree with Dale that if we want to redirect all http://www.youtube.com/* urls to the youtube app or to the video app, we are in feature land and that will not happen in 1.1. I have strong doubts about that being a good idea anyway.
i think this bug is about YouTube app from e.me
if you try to use the share feature from YouTube app, you will be presented with options to share via SMS/Email...etc
so some of that option does not work at all (see attachments)
Google+, Twitter and Facebook all redirect to the browser app. Email opens the email app as expected. Only SMS doesn't work it seems.

We should still reach out to Youtube to get them to use a "share" activity instead.
Needinfo request for Product team feedback on this.

Also, is comment #10 correct, that this is about Youtube in E.me?
Flags: needinfo?(ffos-product)
I think the key issue here is that Youtube believes FirefoxOS supports sms: uri scheme but it seems FirefoxOS browser does not seem to support so. 

For example, when you select "Share through e-mail" youtube uses the mailto uri scheme to invoke e-mail composer and everything works properly. Youtube tries to do the same for sms, invoking a URL with format sms:xxx...

I don't know if supporting the sms URI scheme in the browser has been discussed before, maybe Ben knows?

Anyhow, I think it is quite late in the game to support a new URI scheme in v1.0.1.
Flags: needinfo?(bfrancis)
(In reply to Daniel Coloma:dcoloma from comment #13)
> I think the key issue here is that Youtube believes FirefoxOS supports sms:
> uri scheme but it seems FirefoxOS browser does not seem to support so. 

See bug 793310.  baku can comment more.
Fabrice says we need to verify the URL becuase bug 793310 added support for sms: protocol handling.
Flags: needinfo?(fabrice)
I've tested in Firefox Nightly modifying the User Agent with:

"Mozilla/5.0 (Linux; U; Android 4.0.3; ko-kr; LG-L160L Build/IML74K) AppleWebkit/534.30 (KHTML, like Gecko) Version/4.0 Mobile Safari/534.30"

clicking on share through SMS leads to "The address was not understood" URL in the navigation bar is sms:?body=Has%20recibido%20un%20v%C3%ADdeo%20de%20YouTube%20http%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3DzgA0RvZ445o%26sns%3Dsms
Yes, we get sms:?body=You%20have%20received%20a%20YouTube%20video!%20http%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3De5jDspIC4hY%26sns%3Dsms

This means that there is no SMS number, so we drop the request on the floor. I think we should let these "no number" requests go through and add a "body" parameter to the sms activity.
Flags: needinfo?(fabrice)
Attached patch gecko patchSplinter Review
Changes to the sms protocol handler to support urls without a number when they have a body.
Assignee: nobody → fabrice
Attachment #730354 - Flags: review?(amarchesini)
Attached patch gaia patch (obsolete) — Splinter Review
This let the sms app starts a new sms with a pre-populated text.
Attachment #730357 - Flags: review?(fbsc)
Here we want, 
SMS/Email , lauch local application. But SMS is nor working now, Email is OK.

Facebook/Twitter, should also consider to lauch the application embedded. As we have them by default. But end user need to lauch browser and log in again, seems not friendly.
Flags: needinfo?(sync-1)
(In reply to buri.blff from comment #20)
> Here we want, 
> SMS/Email , lauch local application. But SMS is nor working now, Email is OK.

The attached patches fix the issue for SMS.

> Facebook/Twitter, should also consider to lauch the application embedded. As
> we have them by default. But end user need to lauch browser and log in
> again, seems not friendly.

Sure, but we can't change that. The main issue there is that Youtube harcodes the sharing targets instead of using an activity.
If we can get YouTube to use an activity, what happens if the application that is being shared to is not present on the device (like Facebook/Twitter)?
Flags: needinfo?(ffos-product) → needinfo?(fabrice)
(In reply to Peter Dolanjski [:pdol] from comment #22)
> If we can get YouTube to use an activity, what happens if the application
> that is being shared to is not present on the device (like Facebook/Twitter)?

When using an activity, it's not up to Youtube to create the list of share targets. The platform will let the user choose from the apps that registered themselves as being able to share stuff. Twitter implemented that for instance.
Flags: needinfo?(fabrice)
I think we should block on the SMS part, it seems like a quick-win for a reasonable partner request which may be re-used in other apps
blocking-b2g: tef? → tef+
Andrew answered the question about sms: URLs (thanks), so clearing needinfo.
Flags: needinfo?(bfrancis)
Comment on attachment 730354 [details] [diff] [review]
gecko patch

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

Looks good!
Attachment #730354 - Flags: review?(amarchesini) → review+
Comment on attachment 730357 [details] [diff] [review]
gaia patch

Due to the proposal le ust to compose a new message without 'phonenumber' (only with body) I would change 'showThreadFromSystemMessage' for adding a new condition when (body && !phoneNumber) instead of modifying the case modified in this patch (due to it need to change MessageManager as well for working as expected).
I would change https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/activity_handler.js#L7 so if 'body' exist, we could go directly to '#new'.

Furthermore I would escape the content due to the param could be a HTML or JS script https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/utils.js#L67
Attached patch gaia patch v2 (obsolete) — Splinter Review
New Gaia patch to address comments.
Attachment #730357 - Attachment is obsolete: true
Attachment #730357 - Flags: review?(fbsc)
Attachment #734833 - Flags: review?(fbsc)
Whiteboard: [apps watch list1]
Some small comments added. Please address them and I think we are done! Thanks
Comment on attachment 734833 [details] [diff] [review]
gaia patch v2

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

::: apps/sms/js/activity_handler.js
@@ +7,3 @@
>    var showAction = function act_action(number) {
> +    // If we only have a body, just trigger a new message.
> +    if (!number && body) {

Here we could add:
var escapedBody = Utils.escapeHTML(activity.source.data.body);
MessageManager.activityBody = body;

And remove from the 'system' message handler.

Furthermore 'body', after 'escaping' could be '' string.

::: apps/sms/js/message_manager.js
@@ +164,5 @@
>          ThreadUI.cleanFields();
> +        // If the message has a body, use it to popuplate the input field.
> +        if (MessageManager.activityBody) {
> +          input.value = MessageManager.activityBody;
> +          MessageManager.activityBody = null;

In the performance meetins we realized that initializing to null at the beginning, and then updating the value, is faster than creating this 'on the fly'. Could we add this to the MessageManager object with an explanation about the goal of this var? Thanks!
Attached patch gaia patch v3Splinter Review
Addressing comments.
Attachment #734833 - Attachment is obsolete: true
Attachment #734833 - Flags: review?(fbsc)
Attachment #735913 - Flags: review?(fbsc)
Comment on attachment 735913 [details] [diff] [review]
gaia patch v3

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

::: apps/sms/js/activity_handler.js
@@ +7,5 @@
>    var showAction = function act_action(number) {
> +    // If we only have a body, just trigger a new message.
> +    if (!number && body) {
> +      var escapedBody = Utils.escapeHTML(body);
> +      if (escapedBody != '') {

If (escapedBody === '') {return;}
MessageManager.activityBody = escapedBody;
window.location.hash = '#new';
return;
Comment on attachment 735913 [details] [diff] [review]
gaia patch v3

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

Please address the comment and R+ from my side!
Attachment #735913 - Flags: review?(fbsc) → review+
https://hg.mozilla.org/mozilla-central/rev/53e7c851a6a7
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → B2G C4 (2jan on)
Uplifted commit 3c63c1dd7b7e0452df5b793d85052c4e60eaacde as:
v1-train: 70002cae443b61af9ec57b900beb580c95fccf37
v1.0.1: b7d097edbf42cdadc2b53c90a1c94753af4703c6
Marking affected for v1.0.1 and b2g18 as the gecko part needs to be uplifted.
Flags: needinfo?(ryanvm)
You can just use checkin-needed in the future
Flags: needinfo?(ryanvm)
Keywords: checkin-needed
mozilla do not want to fix this issue.
(In reply to sync-1 from comment #42)
> mozilla do not want to fix this issue.

not sure what you mean, since we fixed it!
can you give me your solution about this issue,I do not found any changes.
We landed a gecko patch and a gaia one to let sms: urls without a number but with a body to work.
There two prs in this issue:first, sharing via sms do not work;second,sharing via sms/email lunch native app,but sharing via facebook/twitter/google+ lunch browser app. About first ,I found you have fixed it,but it seems do not work;About second,I do not found any changes.
(In reply to sync-1 from comment #46)
> There two prs in this issue:first, sharing via sms do not
> work;second,sharing via sms/email lunch native app,but sharing via
> facebook/twitter/google+ lunch browser app. About first ,I found you have
> fixed it,but it seems do not work;About second,I do not found any changes.

The first issue is fixed, I just verified. For the second, you are right that there are no changes since this is the correct behavior (youtube just open links to a blank target).
ok,I see.
Unable to test on Buri device. Marking as QARegressExclude.
Whiteboard: [apps watch list1] → [apps watch list1], QARegressExclude
Why do you unable to test on Buri device? What's the phenomenon?
We have no Buri device to test with here.
(In reply to sync-1 from comment #50)
> Why do you unable to test on Buri device? What's the phenomenon?

It's not device specific, so you can verify on any device. Just check that the Share>Sms functionality in Youtube works.
This issue is not fixed.

When sending and receiving SMS messages with Youtube links on the Inari, the links do not work. When the user attempts to click on the links, neither the browser or the youtube app launches.

When launching a youtube link sent to an email address on the Inari via the Youtube app, when the user selects the link, the video will open in the browser(not the app) 

Inari Build ID: 20130503070205
Environmental  Variables:
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/3f3489356bbc
Gaia: 3e232bce289c9e156d92553e752616cba284bc8f

Attempting to share a video via any of the options available on the Youtube app it is difficult because the options to not always open. For example, the user is attempting to share a link on the Inari and wants to send it via SMS, the user will have to select SMS more than once to prompt the Messenger app. This is the same for all the other options as well. 

Also, when trying to share a link via SMS on the Unagi after already previously sending one, the user will have to close the messenger task in order for a new text to appear with the youtube link they wish to share. Otherwise if the user tries to share the video, the previous text that was last send will appear.

Unagi Build ID: 20130503070205
Environmental  Variables:
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/3f3489356bbc
Gaia: 3e232bce289c9e156d92553e752616cba284bc8f
Now the question is "Tap share from SMS,it come to the Message page,however it can not write anything".Have you see it?
Hi,do you notice this bug --"Tap share from SMS,it come to the Message
page,however it can not create a new message". I think you can turn to new
message page directly.
Hi,have you notice this bug?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: