moebius#56: Fix: DataTransfer - Pasting image from clipboard fails in some cases

https://github.com/MoonchildProductions/moebius/pull/56

devtools/client/framework/test/browser_toolbox_textbox_context_menu.js

@@ -36,15 +36,12 @@ add_task(function* () {
   is(cmdDelete.getAttribute("disabled"), "true", "cmdDelete is disabled");
   is(cmdSelectAll.getAttribute("disabled"), "true", "cmdSelectAll is disabled");
 
-  // Cut/Copy items are enabled in context menu even if there
-  // is no selection. See also Bug 1303033
+  // Cut/Copy/Paste items are enabled in context menu even if there
+  // is no selection. See also Bug 1303033, and 1317322
   is(cmdCut.getAttribute("disabled"), "", "cmdCut is enabled");
   is(cmdCopy.getAttribute("disabled"), "", "cmdCopy is enabled");
 
-  if (isWindows()) {
-    // emptyClipboard only works on Windows (666254), assert paste only for this OS.
-    is(cmdPaste.getAttribute("disabled"), "true", "cmdPaste is disabled");
-  }
+  is(cmdPaste.getAttribute("disabled"), "", "cmdPaste is enabled");
 
   yield cleanup(toolbox);
 });

devtools/client/inspector/computed/test/browser_computed_search-filter_context-menu.js

@@ -45,15 +45,12 @@ add_task(function* () {
   is(cmdDelete.getAttribute("disabled"), "true", "cmdDelete is disabled");
   is(cmdSelectAll.getAttribute("disabled"), "true", "cmdSelectAll is disabled");
 
-  // Cut/Copy items are enabled in context menu even if there
-  // is no selection. See also Bug 1303033
+  // Cut/Copy/Paste items are enabled in context menu even if there is no
+  // selection. See also Bug 1303033, and 1317322
   is(cmdCut.getAttribute("disabled"), "", "cmdCut is enabled");
   is(cmdCopy.getAttribute("disabled"), "", "cmdCopy is enabled");
 
-  if (isWindows()) {
-    // emptyClipboard only works on Windows (666254), assert paste only for this OS.
-    is(cmdPaste.getAttribute("disabled"), "true", "cmdPaste is disabled");
-  }
+  is(cmdPaste.getAttribute("disabled"), "", "cmdPaste is enabled");
 
   info("Closing context menu");
   let onContextMenuHidden = once(searchContextMenu, "popuphidden");

devtools/client/inspector/rules/test/browser_rules_search-filter_context-menu.js

@@ -44,15 +44,12 @@ add_task(function* () {
   is(cmdDelete.getAttribute("disabled"), "true", "cmdDelete is disabled");
   is(cmdSelectAll.getAttribute("disabled"), "true", "cmdSelectAll is disabled");
 
-  // Cut/Copy items are enabled in context menu even if there
-  // is no selection. See also Bug 1303033
+  // Cut/Copy/Paste items are enabled in context menu even if there is no
+  // selection. See also Bug 1303033, and 1317322
   is(cmdCut.getAttribute("disabled"), "", "cmdCut is enabled");
   is(cmdCopy.getAttribute("disabled"), "", "cmdCopy is enabled");
 
-  if (isWindows()) {
-    // emptyClipboard only works on Windows (666254), assert paste only for this OS.
-    is(cmdPaste.getAttribute("disabled"), "true", "cmdPaste is disabled");
-  }
+  is(cmdPaste.getAttribute("disabled"), "", "cmdPaste is enabled");
 
   info("Closing context menu");
   let onContextMenuHidden = once(searchContextMenu, "popuphidden");

devtools/client/inspector/test/browser_inspector_search-filter_context-menu.js

@@ -43,14 +43,11 @@ add_task(function* () {
   is(cmdSelectAll.getAttribute("disabled"), "true", "cmdSelectAll is disabled");
 
   // Cut/Copy items are enabled in context menu even if there
-  // is no selection. See also Bug 1303033
+  // is no selection. See also Bug 1303033, and 1317322
   is(cmdCut.getAttribute("disabled"), "", "cmdCut is enabled");
   is(cmdCopy.getAttribute("disabled"), "", "cmdCopy is enabled");
 
-  if (isWindows()) {
-    // emptyClipboard only works on Windows (666254), assert paste only for this OS.
-    is(cmdPaste.getAttribute("disabled"), "true", "cmdPaste is disabled");
-  }
+  is(cmdPaste.getAttribute("disabled"), "", "cmdPaste is enabled");
 
   info("Closing context menu");
   let onContextMenuHidden = once(searchContextMenu, "popuphidden");

editor/libeditor/HTMLEditorDataTransfer.cpp

@@ -1538,6 +1538,13 @@ HTMLEditor::CanPaste(int32_t aSelectionType,
   NS_ENSURE_ARG_POINTER(aCanPaste);
   *aCanPaste = false;
 
+  // Always enable the paste command when inside of a HTML or XHTML document.
+  nsCOMPtr<nsIDocument> doc = GetDocument();
+  if (doc && doc->IsHTMLOrXHTML()) {
+    *aCanPaste = true;
+    return NS_OK;
+  }
+
   // can't paste if readonly
   if (!IsModifiable()) {
     return NS_OK;

editor/libeditor/TextEditorDataTransfer.cpp

@@ -383,6 +383,13 @@ TextEditor::CanPaste(int32_t aSelectionType,
   NS_ENSURE_ARG_POINTER(aCanPaste);
   *aCanPaste = false;
 
+  // Always enable the paste command when inside of a HTML or XHTML document.
+  nsCOMPtr<nsIDocument> doc = GetDocument();
+  if (doc && doc->IsHTMLOrXHTML()) {
+    *aCanPaste = true;
+    return NS_OK;
+  }
+
   // can't paste if readonly
   if (!IsModifiable()) {
     return NS_OK;

editor/libeditor/tests/chrome.ini

@@ -12,3 +12,4 @@ support-files = green.png
 [test_set_document_title_transaction.html]
 [test_texteditor_keyevent_handling.html]
 skip-if = (debug && os=='win') || (os == 'linux') # Bug 1116205, leaks on windows debug, fails delete key on linux
+[test_pasteImgTextarea.xul]

editor/libeditor/tests/mochitest.ini

@@ -247,3 +247,5 @@ skip-if = toolkit == 'android'
 [test_css_chrome_load_access.html]
 skip-if = toolkit == 'android' # chrome urls not available due to packaging
 [test_selection_move_commands.html]
+[test_pasteImgTextarea.html]
+skip-if = toolkit == 'android' # bug 1299578

editor/libeditor/tests/test_pasteImgTextarea.html

@@ -0,0 +1,20 @@
+<!doctype html>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<script src="/tests/SimpleTest/SpawnTask.js"></script>
+<img id="i" src="green.png">
+<textarea id="t"></textarea>
+
+<script>
+let loaded = new Promise(resolve => addLoadEvent(resolve));
+    add_task(function*() {
+        yield loaded;
+        SpecialPowers.setCommandNode(window, document.getElementById("i"));
+        SpecialPowers.doCommand(window, "cmd_copyImageContents");
+        let input = document.getElementById("t");
+        input.focus();
+        var controller =
+          SpecialPowers.wrap(input).controllers.getControllerForCommand("cmd_paste");
+        is(controller.isCommandEnabled("cmd_paste"), true,
+           "paste should be enabled in html textareas when an image is on the clipboard");
+    });
+</script>

editor/libeditor/tests/test_pasteImgTextarea.xul

@@ -0,0 +1,27 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
+<window xmlns:html="http://www.w3.org/1999/xhtml"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SpawnTask.js"></script>
+
+  <body xmlns="http://www.w3.org/1999/xhtml">
+    <html:img id="i" src="green.png" />
+    <html:textarea id="t"></html:textarea>
+  </body>
+  <script type="text/javascript"><![CDATA[
+    let loaded = new Promise(resolve => addLoadEvent(resolve));
+    add_task(function*() {
+        yield loaded;
+        SpecialPowers.setCommandNode(window, document.getElementById("i"));
+        SpecialPowers.doCommand(window, "cmd_copyImageContents");
+        let input = document.getElementById("t");
+        input.focus();
+        var controller =
+          SpecialPowers.wrap(input).controllers.getControllerForCommand("cmd_paste");
+        is(controller.isCommandEnabled("cmd_paste"), false,
+           "paste should not be enabled in xul textareas when an image is on the clipboard");
+    });
+  ]]></script>
+</window>

widget/windows/nsClipboard.cpp

@@ -65,7 +65,7 @@ nsClipboard::nsClipboard() : nsBaseClipboard()
   nsCOMPtr<nsIObserverService> observerService =
     do_GetService("@mozilla.org/observer-service;1");
   if (observerService)
-    observerService->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, PR_FALSE);
+    observerService->AddObserver(this, NS_XPCOM_WILL_SHUTDOWN_OBSERVER_ID, PR_FALSE);
 }
 
 //-------------------------------------------------------------------------

widget/windows/nsDataObj.cpp

@@ -443,6 +443,82 @@ STDMETHODIMP_(ULONG) nsDataObj::AddRef()
 	return m_cRef;
 }
 
+namespace {
+class RemoveTempFileHelper : public nsIObserver
+{
+public:
+  explicit RemoveTempFileHelper(nsIFile* aTempFile)
+    : mTempFile(aTempFile)
+  {
+    MOZ_ASSERT(mTempFile);
+  }
+
+  // The attach method is seperate from the constructor as we may be addref-ing
+  // ourself, and we want to be sure someone has a strong reference to us.
+  void Attach()
+  {
+    // We need to listen to both the xpcom shutdown message and our timer, and
+    // fire when the first of either of these two messages is received.
+    nsresult rv;
+    mTimer = do_CreateInstance(NS_TIMER_CONTRACTID, &rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return;
+    }
+    mTimer->Init(this, 500, nsITimer::TYPE_ONE_SHOT);
+
+    nsCOMPtr<nsIObserverService> observerService =
+      do_GetService("@mozilla.org/observer-service;1");
+    if (NS_WARN_IF(!observerService)) {
+      mTimer->Cancel();
+      mTimer = nullptr;
+      return;
+    }
+    observerService->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false);
+  }
+
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIOBSERVER
+
+private:
+  ~RemoveTempFileHelper()
+  {
+    if (mTempFile) {
+      mTempFile->Remove(false);
+    }
+  }
+
+  nsCOMPtr<nsIFile> mTempFile;
+  nsCOMPtr<nsITimer> mTimer;
+};
+
+NS_IMPL_ISUPPORTS(RemoveTempFileHelper, nsIObserver);
+
+NS_IMETHODIMP
+RemoveTempFileHelper::Observe(nsISupports* aSubject, const char* aTopic, const char16_t* aData)
+{
+  // Let's be careful and make sure that we don't die immediately
+  RefPtr<RemoveTempFileHelper> grip = this;
+
+  // Make sure that we aren't called again by destroying references to ourself.
+  nsCOMPtr<nsIObserverService> observerService =
+    do_GetService("@mozilla.org/observer-service;1");
+  if (observerService) {
+    observerService->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
+  }
+
+  if (mTimer) {
+    mTimer->Cancel();
+    mTimer = nullptr;
+  }
+
+  // Remove the tempfile
+  if (mTempFile) {
+    mTempFile->Remove(false);
+    mTempFile = nullptr;
+  }
+  return NS_OK;
+}
+} // namespace
 
 //-----------------------------------------------------
 STDMETHODIMP_(ULONG) nsDataObj::Release()
@@ -456,17 +532,12 @@ STDMETHODIMP_(ULONG) nsDataObj::Release()
   // We have released our last ref on this object and need to delete the
   // temp file. External app acting as drop target may still need to open the
   // temp file. Addref a timer so it can delay deleting file and destroying
-  // this object. Delete file anyway and destroy this obj if there's a problem.
+  // this object.
   if (mCachedTempFile) {
-    nsresult rv;
-    mTimer = do_CreateInstance(NS_TIMER_CONTRACTID, &rv);
-    if (NS_SUCCEEDED(rv)) {
-      mTimer->InitWithFuncCallback(nsDataObj::RemoveTempFile, this,
-                                   500, nsITimer::TYPE_ONE_SHOT);
-      return AddRef();
-    }
-    mCachedTempFile->Remove(false);
+    RefPtr<RemoveTempFileHelper> helper =
+      new RemoveTempFileHelper(mCachedTempFile);
     mCachedTempFile = nullptr;
+    helper->Attach();
   }
 
 	delete this;
@@ -2154,13 +2225,3 @@ HRESULT nsDataObj::GetFileContents_IStream(FORMATETC& aFE, STGMEDIUM& aSTG)
 
   return S_OK;
 }
-
-void nsDataObj::RemoveTempFile(nsITimer* aTimer, void* aClosure)
-{
-  nsDataObj *timedDataObj = static_cast<nsDataObj *>(aClosure);
-  if (timedDataObj->mCachedTempFile) {
-    timedDataObj->mCachedTempFile->Remove(false);
-    timedDataObj->mCachedTempFile = nullptr;
-  }
-  timedDataObj->Release();
-}

widget/windows/nsDataObj.h

@@ -289,7 +289,6 @@ protected:
 
     bool LookupArbitraryFormat(FORMATETC *aFormat, LPDATAENTRY *aDataEntry, BOOL aAddorUpdate);
     bool CopyMediumData(STGMEDIUM *aMediumDst, STGMEDIUM *aMediumSrc, LPFORMATETC aFormat, BOOL aSetData);
-    static void RemoveTempFile(nsITimer* aTimer, void* aClosure);
 };