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

VERIFIED FIXED in Firefox 28

Status

()

defect
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: aaronmt, Assigned: wesj)

Tracking

({reproducible, testcase})

28 Branch
Firefox 29
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox28+ fixed, firefox29 verified, fennec28+)

Details

()

Attachments

(1 attachment)

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
Posted 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... http://androidxref.com/4.2.2_r1/xref/frameworks/base/services/java/com/android/server/ClipboardService.java#119

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]
Patch

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/Clipboard.java
@@ +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+
https://hg.mozilla.org/integration/fx-team/rev/235817065969

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]
Patch

[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?
https://hg.mozilla.org/mozilla-central/rev/235817065969
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Status: RESOLVED → VERIFIED
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.