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

VERIFIED FIXED in mozilla21

Status

()

Firefox for Android
Android Sync
P2
normal
VERIFIED FIXED
5 years ago
2 months ago

People

(Reporter: rnewman, Assigned: nalexander)

Tracking

unspecified
mozilla21
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sync:scale][sync:tabs][sendtab][qa+])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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

5 years ago
Sync log, please. I'm specifically interested in the clients engine entries.
Flags: needinfo?(dtownsend+bugmail)
Created attachment 669684 [details]
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?
Flags: needinfo?(dtownsend+bugmail)

Comment 3

5 years ago
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.
(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

5 years ago
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.
Component: Firefox Sync: Backend → Android Sync
(Reporter)

Comment 6

5 years ago
(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.
Summary: Sent tab failure → Send Tab share intent handler produces nonsense commands when capturing intents from Twitter
(Reporter)

Comment 7

5 years ago
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.
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.
(Reporter)

Comment 9

5 years ago
(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?
Looks like with no url you just get the url of the tweet, with two you just get the first url.
(Reporter)

Updated

5 years ago
Priority: -- → P2
Whiteboard: [sync:scale][sync:tabs][sendtab]
(Reporter)

Updated

5 years ago
Duplicate of this bug: 828053
(Reporter)

Comment 12

5 years ago
https://github.com/mozilla-services/android-sync/pull/280
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
https://hg.mozilla.org/integration/mozilla-inbound/rev/602e2fa9589b
Dear QA, please verify that this works with a few Twitter shares.  Thanks!
Keywords: qawanted
Target Milestone: --- → mozilla21
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.
Keywords: qawanted
Whiteboard: [sync:scale][sync:tabs][sendtab] → [sync:scale][sync:tabs][sendtab][qa+]
(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.
> 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.
https://hg.mozilla.org/mozilla-central/rev/602e2fa9589b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED

Updated

5 years ago
Duplicate of this bug: 856489
Component: Android Sync → Android Sync
Product: Mozilla Services → Android Background Services

Updated

2 months ago
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.