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

VERIFIED FIXED in Firefox 29

Status

()

defect
VERIFIED FIXED
6 years ago
2 years ago

People

(Reporter: xyuan, Assigned: rnewman)

Tracking

Firefox 26
Firefox 29
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

57 bytes, text/x-github-pull-request
nalexander
: review+
Details | Review
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.
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.
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.
(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.
(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 :)
Whiteboard: [mentor=rnewman][lang=javascript][good first bug]
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
That's awesome, yxl. I'll give that a shot some time over the holidays, if nobody else gets to it first.
@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>
(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>
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.
OK, I'm convinced. Thanks for doing the legwork!
Posted file Proposed patch. v1
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Attachment #8355315 - Flags: review?(nalexander)
Comment on attachment 8355315 [details] [review]
Proposed patch. v1

r=trivial :)
Attachment #8355315 - Flags: review?(nalexander) → review+
https://hg.mozilla.org/integration/fx-team/rev/daa2a0781fec

Yuan: feel free to propose this for uplift (and verify, too!)
Whiteboard: [mentor=rnewman][lang=javascript][good first bug]
Target Milestone: --- → Firefox 29
@Richard, thanks for your help and it works for me:-)
It's not a critical issue, so targeting 29 is fine for us.
https://hg.mozilla.org/mozilla-central/rev/daa2a0781fec
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Marking verified per Comment 15. Thanks!
Status: RESOLVED → VERIFIED
Hardware: ARM → All
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.