Closed
Bug 590349
Opened 14 years ago
Closed 14 years ago
Clipboard (copy/paste) support for Android
Categories
(Core Graveyard :: Widget: Android, defect)
Tracking
(fennec2.0+)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0+ | --- |
People
(Reporter: mbrubeck, Assigned: blassey)
References
Details
Attachments
(1 file, 2 obsolete files)
13.97 KB,
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
Support the Android clipboard, so that we can implement copy/cut/paste for Fennec. See bug 582912 for some front-end patches.
Reporter | ||
Comment 1•14 years ago
|
||
Sorry, front-end patch has moved to bug 585875.
Reporter | ||
Updated•14 years ago
|
Comment 2•14 years ago
|
||
Is this bug about nsIClipboard support on Android? And any front-end features would be exposed in a bug like bug 585875?
Assignee | ||
Comment 3•14 years ago
|
||
yes, this bug is for supporting the OS clipboard on android through nsIClipboard
Assignee | ||
Comment 4•14 years ago
|
||
Assignee: nobody → blassey.bugs
Attachment #469369 -
Flags: review?(mwu)
Assignee | ||
Comment 5•14 years ago
|
||
Attachment #469369 -
Attachment is obsolete: true
Attachment #469370 -
Flags: review?(mwu)
Attachment #469369 -
Flags: review?(mwu)
Comment 6•14 years ago
|
||
Comment on attachment 469370 [details] [diff] [review] patch (this time with the new files) Nice. How did you test this? >diff --git a/embedding/android/GeckoAppShell.java b/embedding/android/GeckoAppShell.java >--- a/embedding/android/GeckoAppShell.java >+++ b/embedding/android/GeckoAppShell.java >@@ -423,4 +423,20 @@ class GeckoAppShell > return false; > } > } >+ >+ static String getClipboardText() { >+ Context context = GeckoApp.surfaceView.getContext(); >+ android.text.ClipboardManager cm = (android.text.ClipboardManager) >+ context.getSystemService(Context.CLIPBOARD_SERVICE); >+ if (!cm.hasText()) >+ return null; >+ return cm.getText().toString(); >+ } >+ >+ static void setClipboardText(String text) { >+ Context context = GeckoApp.surfaceView.getContext(); >+ android.text.ClipboardManager cm = (android.text.ClipboardManager) >+ context.getSystemService(Context.CLIPBOARD_SERVICE); >+ cm.setText(text); >+ } > } import android.text.*; >diff --git a/widget/src/android/AndroidBridge.cpp b/widget/src/android/AndroidBridge.cpp >--- a/widget/src/android/AndroidBridge.cpp >+++ b/widget/src/android/AndroidBridge.cpp >@@ -360,6 +362,48 @@ AndroidBridge::MoveTaskToBack() > mJNIEnv->CallStaticVoidMethod(mGeckoAppShellClass, jMoveTaskToBack); > } > >+PRBool >+AndroidBridge::GetClipboardText(nsAString& aText) >+{ >+ jstring jstrType = >+ static_cast<jstring>(mJNIEnv-> >+ CallStaticObjectMethod(mGeckoAppShellClass, >+ jGetClipboardText)); >+ if (!jstrType) >+ return PR_FALSE; >+ nsJNIString jniStr(jstrType); >+ aText.Assign(jniStr); >+ return PR_TRUE; >+} >+ >+void >+AndroidBridge::SetClipboardText(const nsAString& aText) >+{ >+ jstring jstr = nsnull; Move this to where it's assigned. >+ const PRUnichar* wText; >+ PRUint32 wTextLen = NS_StringGetData(aText, &wText); This doesn't look safe given the existence of nsSubstringTuple.. but it looks like a bunch of other code do this, so ok.. >diff --git a/widget/src/android/AndroidBridge.h b/widget/src/android/AndroidBridge.h >--- a/widget/src/android/AndroidBridge.h >+++ b/widget/src/android/AndroidBridge.h >@@ -126,6 +126,14 @@ public: > > void MoveTaskToBack(); > >+ PRBool GetClipboardText(nsAString& aText); >+ >+ void SetClipboardText(const nsAString& aText); >+ >+ void EmptyClipboard(); >+ >+ PRBool ClipboardHasText(); >+ Let's start switching to bool. >+NS_IMETHODIMP >+nsClipboard::SetData(nsITransferable *aTransferable, >+ nsIClipboardOwner *anOwner, PRInt32 aWhichClipboard) >+{ >+ if (aWhichClipboard != kGlobalClipboard) >+ return NS_ERROR_NOT_IMPLEMENTED; >+ nsCOMPtr<nsISupports> tmp; >+ PRUint32 len; >+ nsresult rv = aTransferable->GetTransferData(kUnicodeMime, getter_AddRefs(tmp), >+ &len); >+ NS_ENSURE_SUCCESS(rv, rv); >+ nsCOMPtr<nsISupportsString> supportsString = do_QueryInterface(tmp); >+ nsString buffer; nsAutoString for strings inside functions. (unless you have good reason to believe your strings will be greater than 64 characters most of the time) >+ supportsString->GetData(buffer); >+ if (supportsString) { So if we actually hit this check.. we should've crashed earlier, no? >+ if (AndroidBridge::Bridge()) Return at the top of the function if there's no bridge. >+ AndroidBridge::Bridge()->SetClipboardText(buffer); >+ } >+ return NS_OK; >+} >+ >+NS_IMETHODIMP >+nsClipboard::GetData(nsITransferable *aTransferable, PRInt32 aWhichClipboard) >+{ >+ if (aWhichClipboard != kGlobalClipboard) >+ return NS_ERROR_NOT_IMPLEMENTED; >+ if (AndroidBridge::Bridge()) { Return early. >+ nsString buffer; nsAutoString >+ if (!AndroidBridge::Bridge()->GetClipboardText(buffer)) >+ return NS_ERROR_UNEXPECTED; >+ >+ nsresult rv; >+ nsCOMPtr<nsISupportsString> dataWrapper = >+ do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID, &rv); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ rv = dataWrapper->SetData(buffer); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ // If our data flavor has already been added, this will fail. But we don't care >+ aTransferable->AddDataFlavor(kUnicodeMime); >+ >+ nsCOMPtr<nsISupports> nsisupportsDataWrapper = Incorrect indentation. Also, I don't think do_QueryInterface to nsISupports is required since nsISupportsString inherits from nsISupports. >+ do_QueryInterface(dataWrapper); >+ rv = aTransferable->SetTransferData(kUnicodeMime, nsisupportsDataWrapper, >+ buffer.Length() * sizeof(PRUnichar)); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ } >+ return NS_OK; >+} >+
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #6) > Comment on attachment 469370 [details] [diff] [review] > patch (this time with the new files) > > Nice. How did you test this? I wrote some js in about:home to grab text from the clipboard and put it in a div and then set new text to the clipboard. I then pasted that into a text message. Everything worked.
Assignee | ||
Comment 8•14 years ago
|
||
(In reply to comment #6) > Comment on attachment 469370 [details] [diff] [review] > patch (this time with the new files) > >+ // If our data flavor has already been added, this will fail. But we don't care > >+ aTransferable->AddDataFlavor(kUnicodeMime); > >+ > >+ nsCOMPtr<nsISupports> nsisupportsDataWrapper = > Incorrect indentation. Also, I don't think do_QueryInterface to nsISupports is > required since nsISupportsString inherits from nsISupports. > leaving that in the patch because I'm following the lead from here: http://mxr.mozilla.org/mozilla-central/source/widget/src/xpwidgets/nsClipboardHelper.cpp#113
Assignee | ||
Comment 9•14 years ago
|
||
Attachment #469370 -
Attachment is obsolete: true
Attachment #469677 -
Flags: review?(mwu)
Attachment #469370 -
Flags: review?(mwu)
Comment 10•14 years ago
|
||
Comment on attachment 469677 [details] [diff] [review] patch r=mwu with s/nsString/nsAutoString/ in nsClipboard::GetData and a space added after "jSetClipboardText," in AndroidBridge::SetClipboardText.
Attachment #469677 -
Flags: review?(mwu) → review+
Assignee | ||
Updated•14 years ago
|
tracking-fennec: --- → 2.0+
Assignee | ||
Comment 11•14 years ago
|
||
landed this almost a month ago: http://hg.mozilla.org/mozilla-central/rev/6f3b20dd902a
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•