951569 - Sending non-URL content from other apps to Firefox Sync shows "There was a problem sending your tab"

Status: VERIFIED FIXED

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

Description

[Yuan Xulei [:yxl] (Leave from May 18)]

When user shares non-URL content (such as a name card with the mine type of text/x-vcard) from any other app, Firefox Sync will be listed in the android "share list". If user select Firefox Sync to receive the shard content, the following error message will be shown:
 "There was a problem sending your tab"

This was regarded as a bug from our partner who wants to ship our android browser.

Steps to reproduce:
1. Install Firefox on any Android phone supporting contact sharing. For example I use SAMSUNG Galaxy s4 (Android 4.2.2).

2. Open the "Phone" app and switch to "Contacts" page.

3. Long tap a contact to show the context menu and select "Share namedcard via" item.

4. A list of app will be shown to receive the contact.

Expected:

Firefox Sync doesn't appear in the app list.

Actual:

Firefox Sync appears in the app list and when user selects it, a toast with error message -"There was a problem sending your tab" pops up.

Comment 1

[Peipei Cheng (needinfo if you need my action)]

I created a video for STR. Please find it here: https://mozilla.box.com/s/wo91f1p7qhevfkqwlbie

Comment 2

[Yuan Xulei [:yxl] (Leave from May 18)]

The following is the android manifest for the sync activity:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/services/manifests/SyncAndroidManifest_activities.xml.in#91

78         <activity
...
86             android:name="org.mozilla.gecko.sync.setup.activities.SendTabActivity" >
87 
88             <intent-filter>
89                 <action android:name="android.intent.action.SEND" />
90                 <category android:name="android.intent.category.DEFAULT" />
91                 <data android:mimeType="text/*" />
92             </intent-filter>
93         </activity>

SendTabActivity is responsible for receiving sharing data and claims to receive all text data matching the mine type of 'text/*' in the manifest.

Since SendTabActivity can only handle plain text data containing a URL and a subject,  if we limit the mine type of the intent filter to 'text/plain', we can prevent SendTabActivity to receive non-plain text data, such as text/x-vcard. Or more radically, we define a private mine type - 'text/vnd.mozilla.tab' for SendTabActivity.

Comment 3

[Richard Newman [:rnewman]]

There's probably a reason why this is the way it is -- it's trying to be broad enough to catch everything that apps might send (e.g., Twitter, Facebook, ...), and some of those might be surprising.

(This is why we can't define a custom type.)

Before we could make any change here, we'd need to know the mime types sent by apps we *do* want to catch.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Yuan Xulei [:yxl] from comment #2)
> The following is the android manifest for the sync activity:
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/services/
> manifests/SyncAndroidManifest_activities.xml.in#91
> 
> 78         <activity
> ...
> 86            
> android:name="org.mozilla.gecko.sync.setup.activities.SendTabActivity" >
> 87 
> 88             <intent-filter>
> 89                 <action android:name="android.intent.action.SEND" />
> 90                 <category android:name="android.intent.category.DEFAULT"
> />
> 91                 <data android:mimeType="text/*" />
> 92             </intent-filter>
> 93         </activity>
> 
> SendTabActivity is responsible for receiving sharing data and claims to
> receive all text data matching the mine type of 'text/*' in the manifest.
> 
> Since SendTabActivity can only handle plain text data containing a URL and a
> subject,  if we limit the mine type of the intent filter to 'text/plain', we
> can prevent SendTabActivity to receive non-plain text data, such as
> text/x-vcard.

I would not be against text/plain, but I'd want to test the major sharing Apps to make sure we don't footgun ourselves.

 Or more radically, we define a private mine type -
> 'text/vnd.mozilla.tab' for SendTabActivity.

Either I don't understand, or this makes no sense: we explicitly want SendTabActivity to be a share target for mainstream apps like Twitter and Facebook.

Comment 5

[Nick Alexander :nalexander [he/him]]

(In reply to Richard Newman [:rnewman] from comment #3)
> There's probably a reason why this is the way it is -- it's trying to be
> broad enough to catch everything that apps might send (e.g., Twitter,
> Facebook, ...), and some of those might be surprising.

I implemented the text scanning code, but I don't know of any reason we catch text/* instead of text/plain (other than history).  I would be open to text/plain after some investigation.  Presumably some data exists for most common sharing Apps?

> (This is why we can't define a custom type.)
> 
> Before we could make any change here, we'd need to know the mime types sent
> by apps we *do* want to catch.

Hmm, I'm the echo here :)

Comment 6

[Yuan Xulei [:yxl] (Leave from May 18)]

I'm trying to get a mine types list sent by popular apps and created a test app to help.

The test app can show the mine type and text content shared from other apps.

App to download: http://yxl.github.io/ShareDataReceiver/ShareDataReceiver.apk

Source code: https://github.com/yxl/ShareDataReceiver

Comment 7

[Richard Newman [:rnewman]]

That's awesome, yxl. I'll give that a shot some time over the holidays, if nobody else gets to it first.

Comment 8

[Yuan Xulei [:yxl] (Leave from May 18)]

@rnewman, thank you :-)

Here is a short list that may help:

Firefox Android - Share Link
  mine type      = text/plain
  extra text     = <empty>
  extra title    = <url>
  extra subject  = <title>
  data string    = <title>

Android Internet (Embedded Browser) - Share Link
  mine type      = text/plain
  extra text     = <empty>
  extra title    = <url>
  extra subject  = <empty>
  data string    = <title>

Opera - Share Link
  mine type      = text/plain
  extra text     = <url>
  extra title    = <empty>
  extra subject  = <empty>
  data string    = <empty>

UC Browser- Share Link
  mine type      = text/plain
  extra text     = <title + url>
  extra title    = <empty>
  extra subject  = <Add to Bookmark>
  data string    = <empty>

Sumsung Email - Share text or URL within the email content
  mine type      = text/x-vcard
  extra text     = <empty>
  extra title    = <text or URL>
  extra subject  = <empty>
  data string    = <empty>

Sumsung Contacts - Share namecard via
  mine type      = text/x-vcard
  extra text     = <empty>
  extra title    = <empty>
  extra subject  = <Share namecard via>
  data string    = <empty>

Comment 9

[Yuan Xulei [:yxl] (Leave from May 18)]

(In reply to Yuan Xulei [:yxl] from comment #8)
> Sumsung Email - Share text or URL within the email content
>   mine type      = text/x-vcard
Sorry, the mine type should be 'text/plain'.
>   extra text     = <empty>
>   extra title    = <text or URL>
>   extra subject  = <empty>
>   data string    = <empty>

Comment 10

[Yuan Xulei [:yxl] (Leave from May 18)]

Twitter - Share
  mine type      = text/plain
  extra text     = <empty>
  extra title    = <twitter content>
  extra subject  = <empty>
  data string    = <empty>

Facebook - I didn't find how to share a link or message with other apps.

Comment 11

[Richard Newman [:rnewman]]

OK, I'm convinced. Thanks for doing the legwork!

Comment 12

[Richard Newman [:rnewman]]

Attachment: Proposed patch. v1

Comment 13

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8355315 [details] [review]
Proposed patch. v1

r=trivial :)

Comment 14

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/daa2a0781fec

Yuan: feel free to propose this for uplift (and verify, too!)

Comment 15

[Yuan Xulei [:yxl] (Leave from May 18)]

@Richard, thanks for your help and it works for me:-)
It's not a critical issue, so targeting 29 is fine for us.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/daa2a0781fec

Comment 17

[Richard Newman [:rnewman]]

Marking verified per Comment 15. Thanks!