Closed Bug 947793 Opened 7 years ago Closed 7 years ago

Text selection action-bar appears on focus of editable elements with no clipboard content; clipboard button does nothing


(Firefox for Android :: Text Selection, defect)

28 Branch
Not set



Firefox 29
Tracking Status
firefox28 + fixed
firefox29 --- verified
fennec 28+ ---


(Reporter: aaronmt, Assigned: wesj)




(Keywords: reproducible, testcase)


(1 file)

Currently, we show the text-selection action-bar on editable elements e.g, <input> & <textarea>, etc when there is no clipboard content.

To reproduce (with nothing in clipboard, reboot device prior):

data:text/html, <input>
data:text/html, <textarea>

Tap one to focus the field, the virtual keyboard is invoked. Tap again, and the selection action-bar appears. When you have no clipboard value, the clipboard icon does nothing when tapped.

In Chrome, the action-bar for text-selection will only appear when one long-taps on the editable area when there is actual text. When there is only a value to paste, the paste floating text word appears for one to tap.

When this happens, we show the clipboard icon when there is nothing to actually paste at all.
Summary: Text selection action-bar appears on focus of editable elements with no clipboard content → Text selection action-bar appears on focus of editable elements with no clipboard content; clipboard button does nothing
Attached patch PatchSplinter Review
I'm not sure of everything that's going on here. i.e. I can't seem to clear the clipboard in Fennec. I can clear it in other apps, but these methods (hasPrimaryClip) always seem to return true in Fennec anyway. Maybe that's because I used Fennec first? and they seem to be per-user which in Android is per-app...

Rebooting my phones makes them return correct values.

Regardless, this seems like a good fix, and made things work here.
Attachment #8344866 - Flags: review?(nchen)
Comment on attachment 8344866 [details] [diff] [review]

Review of attachment 8344866 [details] [diff] [review]:

Do you know why |getText() != null| didn't work? Otherwise the patch LGTM with nits.

::: mobile/android/base/util/
@@ +86,5 @@
>       */
>      @WrapElementForJNI
>      public static boolean hasText() {
> +        if (Build.VERSION.SDK_INT >= 11) {
> +            android.content.ClipboardManager cm = getClipboardManager11(mContext);

Can you also change "getClipboardManager11" and "getClipboardManager" to have better names? 

Maybe change "getClipboardManager" to "getDeprecatedClipboardManager",
and change "getClipboardManager11" to "getClipboardManager"
Attachment #8344866 - Flags: review?(nchen) → review+

Reading through this a bit closer, the problem is that our getText implementation doesn't return null if it doesn't find anything. It intentionally returns an empty string. We could change that, or change to use TextUtils.isEmpty(). But I don't see any reason not to just query the manager directly either.

Interestingly, Android's own implementation changed between 2.3 and 4.0. Prior to 4.0 it would always return empty strings as well. Afterwards it returns null if the clipboard is empty.
Comment on attachment 8344866 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 91398.
User impact if declined: Paste shows up as an option even if it doesn't apply.
Testing completed (on m-c, etc.): Landed on mc today
Risk to taking this patch (and alternatives if risky): Low risk. Fixing a regression that (sadly) our tests didn't catch when the generated jni stuff landed.
String or IDL/UUID changes made by this patch: none.
Attachment #8344866 - Flags: approval-mozilla-aurora?
Assignee: nobody → wjohnston
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
tracking-fennec: ? → 28+
Attachment #8344866 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.