Closed Bug 1449504 Opened 7 years ago Closed 7 years ago

Crash/no action when sharing large amount of text from Text Selection Action Bar

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox60 wontfix, firefox61 verified)

VERIFIED FIXED
Firefox 61
Tracking Status
firefox60 --- wontfix
firefox61 --- verified

People

(Reporter: sflorean, Assigned: JanH)

Details

Attachments

(2 files)

Environment: Device: Nexus 5(Android 6.0.1), Huawei MediaPad M2 (Android 5.1.1); Build: Nightly 61.0a1 (2018-03-27); Steps to reproduce: 1. Launch Firefox and go to planet.mozilla.org; 2. Long tap on a word and press "select all"; 3. Tap on "share" from context menu/action bar. Expected result: The text is shared through that option you choose from the list displayed with available apps. Actual result: Firefox crashes (for Android 6+). Action bar is dismiss after tapping on share and text is not shared and is not selected anymore (for Android 5). Crash: https://crash-stats.mozilla.com/report/index/30c238c0-d1cf-4736-8e1a-22ab20180328
Assignee: nobody → jh+bugzilla
Hardware: ARM → All
Summary: Crash/no action when sharing from Action Bar → Crash/no action when sharing large amount of text from Text Selection Action Bar
Right now, planet.mozilla.org has ~330k characters. Java uses stores strings as UTF-16 and when launching an ACTION_SEND intent, the EXTRA_TEXT contents are copied into the Intent's ClipData as well, doubling memory requirements once more. On the other hand according to [1] we can send at most 1 MB via an intent, and in practice even less because that megabyte needs to be shared with any other concurrently running Binder transactions. Chrome truncates to 100k chars [2]. [1] https://developer.android.com/reference/android/os/TransactionTooLargeException.html [2] https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java?l=78&rcl=b3e039d5ba5925eefdc46f4c08cea7e02b997aa3
Comment on attachment 8963717 [details] Bug 1449504 - Part 1 - Truncate overly long text before sharing via an Intent. https://reviewboard.mozilla.org/r/232610/#review238306 ::: mobile/android/base/java/org/mozilla/gecko/IntentHelper.java:211 (Diff revision 2) > public static Intent getShareIntent(final Context context, > - final String targetURI, > + String targetURI, > final String mimeType, > final String title) { > + if (!TextUtils.isEmpty(targetURI) && targetURI.length() > MAX_INTENT_STRING_DATA_LENGTH) { > + targetURI = targetURI.substring(0, MAX_INTENT_STRING_DATA_LENGTH) + "…"; Use something like `context.getString(R.string.ellipsis)` because the ellipsis character depends on locale. ::: mobile/android/base/java/org/mozilla/gecko/IntentHelper.java:215 (Diff revision 2) > + if (!TextUtils.isEmpty(targetURI) && targetURI.length() > MAX_INTENT_STRING_DATA_LENGTH) { > + targetURI = targetURI.substring(0, MAX_INTENT_STRING_DATA_LENGTH) + "…"; > + } > + > Intent shareIntent = getIntentForActionString(Intent.ACTION_SEND); > shareIntent.putExtra(Intent.EXTRA_TEXT, targetURI); This patch is fine, though a proper solution is probably to switch to using `EXTRA_STREAM`.
Attachment #8963717 - Flags: review?(nchen) → review+
Comment on attachment 8963717 [details] Bug 1449504 - Part 1 - Truncate overly long text before sharing via an Intent. https://reviewboard.mozilla.org/r/232610/#review238306 > This patch is fine, though a proper solution is probably to switch to using `EXTRA_STREAM`. I can file a follow-up bug for that, but just to be sure that I'm thinking the same thing that you're thinking: You mean something like storing the whole text in a temporary file and then sharing an Uri to that file using EXTRA_STREAM? (And then of course cleaning up that file again some time, maybe on the next app launch - I seem to remember we already have some helper function for cleaning up old unused files)
Attachment #8963716 - Flags: review+
Pushed by mozilla@buttercookie.de: https://hg.mozilla.org/integration/autoland/rev/c283a92d8471 Part 0 - Cleanup unused imports. r=JanH https://hg.mozilla.org/integration/autoland/rev/998fb1b80384 Part 1 - Truncate overly long text before sharing via an Intent. r=jchen
Comment on attachment 8963717 [details] Bug 1449504 - Part 1 - Truncate overly long text before sharing via an Intent. https://reviewboard.mozilla.org/r/232610/#review238306 > I can file a follow-up bug for that, but just to be sure that I'm thinking the same thing that you're thinking: You mean something like storing the whole text in a temporary file and then sharing an Uri to that file using EXTRA_STREAM? (And then of course cleaning up that file again some time, maybe on the next app launch - I seem to remember we already have some helper function for cleaning up old unused files) Or I suppose actually implementing a full-blown ContentProvider that would temporarily hold on to the full text and send it out if requested, similar to what was e.g. proposed way back in bug 649371.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Verified as fixed following the steps from description on latest Nightly build (2018-04-04). Devices: Nexus 5(Android 6.0.1) and Huawei MediaPad M2 (Android 5.1.1).
tracking-fennec: ? → ---
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: