1014605 - [Browser][V2.0] Cannot add a RTSP shortcut on homescreen

Status: RESOLVED FIXED

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

Description

[William Hsu [:whsu]]

Attachment: WP_20140522_012.mp4

* Description:
  Nothing happened after trying to add a RTSP shortcut via "Add to homescreen" button
  Attach the demo video.(WP_20140522_012.mp4)

* Reproduction steps:
  1. Visit following webpage - http://goo.gl/BhJd77
  2. Tap "RTSP test page" link
  3. Tap star icon
  4. Tap "Add to home screen"

* Expected result:
  You can add a shortcut on homescreen

* Actual result:
  Nothing happened

* Reproduction build: V2.0 (Buri)
 - Gaia      40abb245b1e61df67757ba3747e2f73202e5182b
 - Gecko     https://hg.mozilla.org/mozilla-central/rev/a7d685480bf2
 - BuildID   20140521160200
 - Version   32.0a1

Comment 1

[Gregor Wagner [:gwagner]]

Is this a regression?

Comment 2

[Jason Smith [:jsmith]]

(In reply to Gregor Wagner [:gwagner] from comment #1)
> Is this a regression?

Might be. But it's a minor bug overall - most users won't add a homescreen bookmark based on a RTSP link.

Comment 6

[yifan [:yifan][:yliao]]

Attachment: pull request for v2.0

'Add to homescreen' MozActivity is handled by the bookmark app. However RTSP urls are not currently registered and handled in MozActivities. The PR adds RTSP url support in the manifest.webapp of the bookmark app.

Comment 7

[Ben Francis [:benfrancis]]

Comment on attachment 8491234 [details] [review]
pull request for v2.0

Flagging bookmark app owner for review.

Comment 8

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8491234 [details] [review]
pull request for v2.0

LGTM and it makes sense but please check if UrlHelper.isNotURL('rtsp://100.100.100.100/rtsp.mp4') is false before landing.

https://github.com/mozilla-b2g/gaia/blob/master/apps/bookmark/js/bookmark_editor.js#L86

Just a question, why the patch is for 2.0? I thought that commits go to master and after that, we have to request for approval-gaia-v2.0 in this case, right?

Comment 9

[Ethan Tseng [:ethan]]

Reset assignee since Yi-Fan already provided the patch.
Thanks everyone for working on this. :)

Comment 10

[yifan [:yifan][:yliao]]

Attachment: pull request

Thank you. :)

Checked RTSP link is valid in the UrlHelper.

Created PR for master branch. File changes are the same.

Comment 11

[yifan [:yifan][:yliao]]

Unit test for rtsp links added.

Comment 12

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8492013 [details] [review]
pull request

Perfect, thanks a lot!

Comment 13

[yifan [:yifan][:yliao]]

Merged into master:
https://github.com/mozilla-b2g/gaia/pull/24203

TBPL:
https://tbpl.mozilla.org/?rev=a7fc490ebb610805a3cd6a3bc502e77a06893892&tree=Gaia-Try

Comment 14

[shawn ku [:sku]]

Hi Yi-Fan:
 Could you please help prepare 2.0m patch once 2.0m+?

Thanks!!
Shawn Ku

Comment 15

[yifan [:yifan][:yliao]]

Attachment: pull request for v2.0, v2.0m, v2.1

Code changes are slightly different from master branch in url_helper_test.js.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 1014605
[User impact] if declined: RSTP links are not able to be added to homescreen
[Testing completed]: 
for v2.0: https://tbpl.mozilla.org/?rev=8bf89eecd3031a23f4274cc316f45bb425fa409c&tree=Gaia-Try
for v2.1: https://tbpl.mozilla.org/?rev=32f5753171da197cd25dea75d541c59fe7abb54e&tree=Gaia-Try
[Risk to taking this patch] (and alternatives if risky): No
[String changes made]: No

Comment 16

[yifan [:yifan][:yliao]]

Comment on attachment 8493485 [details] [review]
pull request for v2.0, v2.0m, v2.1

Yes, attachment 8493485 [details] [review] is also for v2.0m.

Comment 17

[shawn ku [:sku]]

Hi Seinlin:
 Please uplift the patch to 2.0m.

Thanks!!
Shawn

Comment 18

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8493485 [details] [review]
pull request for v2.0, v2.0m, v2.1

The code is the same and looks good too :), I guess that once received the approval+ your patch with r+ will be uplifted. you don't need more patches here. Please ask for approval for your patch which is already landed on master. Thanks a lot for your help.

Comment 19

[Cristian Rodriguez (:crdlc)]

If the code changes, you have to resolve conflicts uplifting, I mean, during the cherry-pick. Thanks Yi-Fan

(In reply to Yi-Fan Liao [:yifan][:yliao] from comment #15)
> Created attachment 8493485 [details] [review]
> pull request for v2.0, v2.0m, v2.1
> 
> Code changes are slightly different from master branch in url_helper_test.js.
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): 1014605
> [User impact] if declined: RSTP links are not able to be added to homescreen
> [Testing completed]: 
> for v2.0:
> https://tbpl.mozilla.org/
> ?rev=8bf89eecd3031a23f4274cc316f45bb425fa409c&tree=Gaia-Try
> for v2.1:
> https://tbpl.mozilla.org/
> ?rev=32f5753171da197cd25dea75d541c59fe7abb54e&tree=Gaia-Try
> [Risk to taking this patch] (and alternatives if risky): No
> [String changes made]: No

Comment 20

[yifan [:yifan][:yliao]]

Thank you! 

Previously I noticed that there are some slight differences of url_helper.js. Testing 'http:' and 'rtsp:' fails in url_helper_test.js before and including v2.1. So the test code for v2.1 and before is different. I tried cherry-pick with v2.0, v2.0m and v2.1 with no conflicts and test failure. Will request uplift with the 2nd pull request.

Comment 21

[Kai-Zhen Li [:seinlin][:kli]]

Yifan, I am not sure if 2.0 will get approval to land or not. But 2.0m need this patch. Can you send PR to v2.0m? Thanks!

Comment 22

[yifan [:yifan][:yliao]]

'pull request for v2.0, v2.0m, v2.1' is the one for you. It could be cherry-picked into v2.0m with no conflicts. I'll leave v2.0 and v2.1 unchanged.

Comment 23

[Kai-Zhen Li [:seinlin][:kli]]

v2.0m: https://github.com/mozilla-b2g/gaia/commit/4a6a36d828f470e2d34b2af5175cebf3f64d1a71

Comment 24

[Lance（leave from Mozilla）]

This issue has been verified successfully on Woodduck 2.0. We use bug 1059929 website  instead of  "http://goo.gl/BhJd77" to verify this bug.
See attachment: 1601.MP4
Reproducing rate: 0/5

Step:
1.Launch Browser.
2.Go to the "rtsp://184.72.239.149/vod/mp4:BigBuckBunny_175k.mov".
3.Tap play icon to play the video.
4.Tap "^" -> star icon.
5.Tap "Add to home screen" -> "Add to Home Screen".
6.Back to Home.
**The shortcut have been added to home.

Woodduck version:
Gaia-Rev        87b23fa81c3b59f2ba24b84f7d686f4160d4e7cb
Gecko-Rev       d049d4ef127844121c9cf14d2e8ca91fd9045fcb
Build-ID        20141125050313
Version         32.0
Device-Name     jrdhz72_w_ff
FW-Release      4.4.2
FW-Incremental  1416863158
FW-Date         Tue Nov 25 05:06:35 CST 2014

Comment 25

[Lance（leave from Mozilla）]

Attachment: 1601.MP4

Comment 27

[Shine(Leave from Mozilla)]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/11041/