799615 - Send Tab share intent handler produces nonsense commands when capturing intents from Twitter

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Android Sync, defect, P2)

Description

[Richard Newman [:rnewman]]

11:07:05 < Mossop> Ok, I got a tab open that has nothing in the url bar and "The address isn't valid" as an error in the page
11:07:27 <@rnewman> exciting 
11:07:41 <@rnewman> what page did you send? 
11:08:23 < Mossop> rnewman: Should have just been this link http://imgur.com/a/lUWTG
11:08:42 < Mossop> I also shared it with pocket and it made it therefine
11:08:50 <@rnewman> can you repro?
11:09:08  * Mossop tries
11:09:28 <@rnewman> thanks
11:09:50 < Mossop> Yep
11:10:05 < Mossop> This is from Firefox release on my phone to Nightly on my desktop if that makes any difference

Comment 1

[Gregory Szorc [:gps]]

Sync log, please. I'm specifically interested in the clients engine entries.

Comment 2

[Dave Townsend [:mossop]]

Attachment: sync log

Looks like twitter is sharing the entire tweet. Every other thing that I try sharing with in the same way correctly gets just the link. Is there something we can do to be more sane here?

Comment 3

[Gregory Szorc [:gps]]

The first part is the page title, which we grab from the tab for UX purposes. We have little control over it.

I guess the next step is tracking down why https://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#1599 isn't working.

Comment 4

[Dave Townsend [:mossop]]

(In reply to Gregory Szorc [:gps] from comment #3)
> The first part is the page title, which we grab from the tab for UX
> purposes. We have little control over it.

What tab? This is coming from the native twitter app.

Comment 5

[Gregory Szorc [:gps]]

Wait, wat? You can do that?!

Well, looking at it again, here's a log entry:

1349809593068	Sync.Engine.Clients	INFO	Received a URI for display: Tim Burgess (@Tim_Burgess) tweeted at 1:20 AM on Tue, Oct 09, 2012:
These signs on the London Underground = pure genius http://t.co/GlCPR5k2 'Aren't you wonderful taking Little Hugo to the museums' Brilliant
(https://twitter.com/Tim_Burgess/status/255583557706186752)

Get the official Twitter app at https://twitter.com/download (Tweet from Tim Burgess (@Tim_Burgess)) from E_NIYsutBVth

Looking at the source code from clients.js:

  _handleDisplayURI: function _handleDisplayURI(uri, clientId, title) {
    this._log.info("Received a URI for display: " + uri + " (" + title +
                   ") from " + clientId);

Looks like the command coming off the Android client isn't properly structured.

There's probably a bug in here for Firefox to validate incoming data better. But, the underlying problem here the record coming from Android is malformed. Bad Android.

Comment 6

[Richard Newman [:rnewman]]

(In reply to Dave Townsend (:Mossop) from comment #4)

> What tab? This is coming from the native twitter app.

That is new and exciting information, which no doubt will make this easier to repro!


(In reply to Gregory Szorc [:gps] from comment #5)

> There's probably a bug in here for Firefox to validate incoming data better.
> But, the underlying problem here the record coming from Android is
> malformed. Bad Android.

Very likely. Renaming this bug.

Comment 7

[Richard Newman [:rnewman]]

We do this:

    final String uri = extras.getString(Intent.EXTRA_TEXT);
    final String title = extras.getString(Intent.EXTRA_SUBJECT);

This is typical of shares.

I hope that Twitter is simply putting some meaningful URL (the URL of the tweet?) in the Intent's data field, and thus we have a one-line change to make here.

Comment 8

[Dave Townsend [:mossop]]

FTR what I want (and what I get from say sharing to Pocket) is to send the url contained in the tweet, not the tweet's url itself. Now I guess it's possible that Pocket and the other share actions I've tried are manually scanning the text for links to use so perhaps we need to do that too.

Comment 9

[Richard Newman [:rnewman]]

(In reply to Dave Townsend (:Mossop) from comment #8)
> FTR what I want (and what I get from say sharing to Pocket) is to send the
> url contained in the tweet, not the tweet's url itself. Now I guess it's
> possible that Pocket and the other share actions I've tried are manually
> scanning the text for links to use so perhaps we need to do that too.

What does Pocket do if there are multiple URLs in the body? Or none?

Comment 10

[Dave Townsend [:mossop]]

Looks like with no url you just get the url of the tweet, with two you just get the first url.

Comment 12

[Richard Newman [:rnewman]]

https://github.com/mozilla-services/android-sync/pull/280

Comment 13

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/602e2fa9589b

Comment 14

[Nick Alexander :nalexander [he/him]]

Dear QA, please verify that this works with a few Twitter shares.  Thanks!

Comment 15

[Tracy Walker [:tracy]]

steps to reproduce please.  What sort of links are you sharing?  I have never used Twitter, please keep that in mind with your explanation. Thank you.

Comment 16

[Nick Alexander :nalexander [he/him]]

(In reply to Tracy Walker [:tracy] from comment #15)
> steps to reproduce please.  What sort of links are you sharing?  I have
> never used Twitter, please keep that in mind with your explanation. Thank
> you.

STR:

1) set up Twitter App on Android device.

2a) set up Sync on desktop device.
2b) pair Sync on Android device.

3a) In Fennec, browse to a site.
3b) Option menu > Share > Firefox Sync
3c) Select desktop device, tap Send.
3d) Verify correct tab is received on desktop.

4) Same as 3, but with Stock Android Browser.

5) Optional: same as 3, but with Google Chrome.

6a) In Twitter App on Android device, view any tweet.
6b) Tap "Share" icon -- looks like two arrows coming out of a circle for me, and is in the far right bottom corner.
6c) Select "Firefox Sync".
6d) Select desktop device, tap Send.
6e) Verify correct tab is received on desktop.

I'm going to run that last through myself right now.

Comment 17

[Nick Alexander :nalexander [he/him]]

> 6a) In Twitter App on Android device, view any tweet.
> 6b) Tap "Share" icon -- looks like two arrows coming out of a circle for me,
> and is in the far right bottom corner.
> 6c) Select "Firefox Sync".
> 6d) Select desktop device, tap Send.
> 6e) Verify correct tab is received on desktop.

"Correct tab" should be whatever URL is *first* linked to in the shared tweet, or the URL of the shared tweet itself if it doesn't link to anything.  We're really concerned about grabbing a link *in* the tweet here.

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/602e2fa9589b

Comment 19

[Tracy Walker [:tracy]]

Verified with nightly build of 20130118.  If tweet doesn't have a link in it, the tweet is opened in new tab on Desktop.  If tweet has a link in it, that link is opened in a new tab instead.