From 4549256c2b685782587f2ccad6a105c0392492c7 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Fri, 20 Apr 2018 22:54:26 +0200 Subject: moebius#56: Fix: DataTransfer - Pasting image from clipboard fails in some cases https://github.com/MoonchildProductions/moebius/pull/56 --- .../test/browser_toolbox_textbox_context_menu.js | 9 +- .../browser_computed_search-filter_context-menu.js | 9 +- .../browser_rules_search-filter_context-menu.js | 9 +- ...browser_inspector_search-filter_context-menu.js | 7 +- editor/libeditor/HTMLEditorDataTransfer.cpp | 7 ++ editor/libeditor/TextEditorDataTransfer.cpp | 7 ++ editor/libeditor/tests/chrome.ini | 1 + editor/libeditor/tests/mochitest.ini | 2 + editor/libeditor/tests/test_pasteImgTextarea.html | 20 +++++ editor/libeditor/tests/test_pasteImgTextarea.xul | 27 ++++++ widget/windows/nsClipboard.cpp | 2 +- widget/windows/nsDataObj.cpp | 99 +++++++++++++++++----- widget/windows/nsDataObj.h | 1 - 13 files changed, 156 insertions(+), 44 deletions(-) create mode 100644 editor/libeditor/tests/test_pasteImgTextarea.html create mode 100644 editor/libeditor/tests/test_pasteImgTextarea.xul diff --git a/devtools/client/framework/test/browser_toolbox_textbox_context_menu.js b/devtools/client/framework/test/browser_toolbox_textbox_context_menu.js index 2e5f3210e..e2e961255 100644 --- a/devtools/client/framework/test/browser_toolbox_textbox_context_menu.js +++ b/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); }); diff --git a/devtools/client/inspector/computed/test/browser_computed_search-filter_context-menu.js b/devtools/client/inspector/computed/test/browser_computed_search-filter_context-menu.js index b5dbe4475..b04a247f5 100644 --- a/devtools/client/inspector/computed/test/browser_computed_search-filter_context-menu.js +++ b/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"); diff --git a/devtools/client/inspector/rules/test/browser_rules_search-filter_context-menu.js b/devtools/client/inspector/rules/test/browser_rules_search-filter_context-menu.js index 349f1b9b3..d66df91ab 100644 --- a/devtools/client/inspector/rules/test/browser_rules_search-filter_context-menu.js +++ b/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"); diff --git a/devtools/client/inspector/test/browser_inspector_search-filter_context-menu.js b/devtools/client/inspector/test/browser_inspector_search-filter_context-menu.js index 137456468..7bd4b5e6f 100644 --- a/devtools/client/inspector/test/browser_inspector_search-filter_context-menu.js +++ b/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"); diff --git a/editor/libeditor/HTMLEditorDataTransfer.cpp b/editor/libeditor/HTMLEditorDataTransfer.cpp index ed350c0dd..c56fbead7 100644 --- a/editor/libeditor/HTMLEditorDataTransfer.cpp +++ b/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 doc = GetDocument(); + if (doc && doc->IsHTMLOrXHTML()) { + *aCanPaste = true; + return NS_OK; + } + // can't paste if readonly if (!IsModifiable()) { return NS_OK; diff --git a/editor/libeditor/TextEditorDataTransfer.cpp b/editor/libeditor/TextEditorDataTransfer.cpp index 0388aa4a8..2cc2906fa 100644 --- a/editor/libeditor/TextEditorDataTransfer.cpp +++ b/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 doc = GetDocument(); + if (doc && doc->IsHTMLOrXHTML()) { + *aCanPaste = true; + return NS_OK; + } + // can't paste if readonly if (!IsModifiable()) { return NS_OK; diff --git a/editor/libeditor/tests/chrome.ini b/editor/libeditor/tests/chrome.ini index 98db30001..dd13370a5 100644 --- a/editor/libeditor/tests/chrome.ini +++ b/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] diff --git a/editor/libeditor/tests/mochitest.ini b/editor/libeditor/tests/mochitest.ini index 4df3f606b..33b164819 100644 --- a/editor/libeditor/tests/mochitest.ini +++ b/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 diff --git a/editor/libeditor/tests/test_pasteImgTextarea.html b/editor/libeditor/tests/test_pasteImgTextarea.html new file mode 100644 index 000000000..3168ae729 --- /dev/null +++ b/editor/libeditor/tests/test_pasteImgTextarea.html @@ -0,0 +1,20 @@ + + + + + + + diff --git a/editor/libeditor/tests/test_pasteImgTextarea.xul b/editor/libeditor/tests/test_pasteImgTextarea.xul new file mode 100644 index 000000000..545027aa3 --- /dev/null +++ b/editor/libeditor/tests/test_pasteImgTextarea.xul @@ -0,0 +1,27 @@ + + + + + + + + + + + + + diff --git a/widget/windows/nsClipboard.cpp b/widget/windows/nsClipboard.cpp index 1afee3496..0db1dd342 100644 --- a/widget/windows/nsClipboard.cpp +++ b/widget/windows/nsClipboard.cpp @@ -65,7 +65,7 @@ nsClipboard::nsClipboard() : nsBaseClipboard() nsCOMPtr 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); } //------------------------------------------------------------------------- diff --git a/widget/windows/nsDataObj.cpp b/widget/windows/nsDataObj.cpp index 02ec3b2fe..977a87c08 100644 --- a/widget/windows/nsDataObj.cpp +++ b/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 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 mTempFile; + nsCOMPtr 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 grip = this; + + // Make sure that we aren't called again by destroying references to ourself. + nsCOMPtr 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 helper = + new RemoveTempFileHelper(mCachedTempFile); mCachedTempFile = nullptr; + helper->Attach(); } delete this; @@ -2153,13 +2224,3 @@ HRESULT nsDataObj::GetFileContents_IStream(FORMATETC& aFE, STGMEDIUM& aSTG) return S_OK; } - -void nsDataObj::RemoveTempFile(nsITimer* aTimer, void* aClosure) -{ - nsDataObj *timedDataObj = static_cast(aClosure); - if (timedDataObj->mCachedTempFile) { - timedDataObj->mCachedTempFile->Remove(false); - timedDataObj->mCachedTempFile = nullptr; - } - timedDataObj->Release(); -} diff --git a/widget/windows/nsDataObj.h b/widget/windows/nsDataObj.h index 2d7fb75ee..61f209e85 100644 --- a/widget/windows/nsDataObj.h +++ b/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); }; -- cgit v1.2.3