diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp index af8eb2336a89e..dcba4a5ea866e 100644 --- a/dom/base/Document.cpp +++ b/dom/base/Document.cpp @@ -1319,6 +1319,7 @@ Document::Document(const char* aContentType) mPendingMaybeEditingStateChanged(false), mHasBeenEditable(false), mHasWarnedAboutZoom(false), + mIsRunningExecCommand(false), mPendingFullscreenRequests(0), mXMLDeclarationBits(0), mOnloadBlockCount(0), @@ -4694,6 +4695,13 @@ bool Document::ExecCommand(const nsAString& aHTMLCommandName, bool aShowUI, return false; } + // If we're running an execCommand, we should just return false. + // https://github.com/w3c/editing/issues/200#issuecomment-575241816 + if (!StaticPrefs::dom_document_exec_command_nested_calls_allowed() && + mIsRunningExecCommand) { + return false; + } + // for optional parameters see dom/src/base/nsHistory.cpp: HistoryImpl::Go() // this might add some ugly JS dependencies? @@ -4790,6 +4798,8 @@ bool Document::ExecCommand(const nsAString& aHTMLCommandName, bool aShowUI, } } + AutoRunningExecCommandMarker markRunningExecCommand(*this); + // If we cannot use EditorCommand instance directly, we need to handle the // command with traditional path (i.e., with DocShell or nsCommandManager). if (!editorCommand) { diff --git a/dom/base/Document.h b/dom/base/Document.h index 5367c7348fb4f..d7cdaf36ae510 100644 --- a/dom/base/Document.h +++ b/dom/base/Document.h @@ -4063,6 +4063,34 @@ class Document : public nsINode, const nsAString& aValue = EmptyString(), nsAString* aAdjustedValue = nullptr); + /** + * AutoRunningExecCommandMarker is AutoRestorer for mIsRunningExecCommand. + * Since it's a bit field, not a bool member, therefore, we cannot use + * AutoRestorer for it. + */ + class MOZ_STACK_CLASS AutoRunningExecCommandMarker final { + public: + AutoRunningExecCommandMarker() = delete; + explicit AutoRunningExecCommandMarker(const AutoRunningExecCommandMarker&) = + delete; + // Guaranteeing the document's lifetime with `MOZ_CAN_RUN_SCRIPT`. + MOZ_CAN_RUN_SCRIPT explicit AutoRunningExecCommandMarker( + Document& aDocument) + : mDocument(aDocument), + mHasBeenRunning(aDocument.mIsRunningExecCommand) { + aDocument.mIsRunningExecCommand = true; + } + ~AutoRunningExecCommandMarker() { + if (!mHasBeenRunning) { + mDocument.mIsRunningExecCommand = false; + } + } + + private: + Document& mDocument; + bool mHasBeenRunning; + }; + // Mapping table from HTML command name to internal command. typedef nsDataHashtable InternalCommandDataHashtable; @@ -4569,6 +4597,9 @@ class Document : public nsINode, // also record this as a `CountedUnknownProperty`. bool mHasWarnedAboutZoom : 1; + // While we're handling an execCommand call, set to true. + bool mIsRunningExecCommand : 1; + uint8_t mPendingFullscreenRequests; uint8_t mXMLDeclarationBits; diff --git a/editor/libeditor/crashtests/crashtests.list b/editor/libeditor/crashtests/crashtests.list index 791d3d4d9ec62..ca010200f8971 100644 --- a/editor/libeditor/crashtests/crashtests.list +++ b/editor/libeditor/crashtests/crashtests.list @@ -91,8 +91,8 @@ load 1393171.html needs-focus load 1402196.html load 1402469.html load 1402526.html -asserts(1) load 1402904.html -asserts(1) load 1405747.html +pref(dom.document.exec_command.nested_calls_allowed,true) asserts(1) load 1402904.html # assertion is that mutation event listener caused by execCommand calls another execCommand +pref(dom.document.exec_command.nested_calls_allowed,true) asserts(1) load 1405747.html # assertion is that mutation event listener caused by execCommand calls another execCommand load 1405897.html load 1408170.html asserts(0-1) load 1414581.html @@ -106,9 +106,9 @@ load 1441619.html load 1443664.html skip-if(Android) needs-focus load 1444630.html load 1446451.html -asserts(0-2) load 1464251.html # assertion is that mutation event listener modifies content +pref(dom.document.exec_command.nested_calls_allowed,true) asserts(2) load 1464251.html # assertion is that mutation event listener caused by execCommand calls another execCommand pref(layout.accessiblecaret.enabled,true) load 1470926.html -asserts(0-1) load 1474978.html # assertion is that mutation event listener modifies content +pref(dom.document.exec_command.nested_calls_allowed,true) asserts(1) load 1474978.html # assertion is that mutation event listener caused by execCommand calls another execCommand load 1525481.html load 1533913.html load 1534394.html diff --git a/editor/libeditor/tests/test_bug1425997.html b/editor/libeditor/tests/test_bug1425997.html index 8000a96d93895..3e3e3e9e6e3fd 100644 --- a/editor/libeditor/tests/test_bug1425997.html +++ b/editor/libeditor/tests/test_bug1425997.html @@ -26,7 +26,7 @@ SimpleTest.waitForExplicitFinish(); // 3 assertions are recorded due to nested execCommand() but not a problem. // They are necessary to detect invalid method call without mutation event listers. -SimpleTest.expectAssertions(3, 3); +SimpleTest.expectAssertions(0, 3); SimpleTest.waitForFocus(function() { let selection = window.getSelection(); let editor = document.getElementById("editor"); @@ -38,7 +38,9 @@ // after 2nd delete: "\n \n" // after 3rd delete: "\n\n" while (editor.innerHTML.includes(" ")) { - document.execCommand("delete", false); + if (!document.execCommand("delete", false)) { + break; + } } } editor.addEventListener("DOMCharacterDataModified", onCharacterDataModified, { once: true }); diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 7865ec0e4cc88..1846da305a390 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -151,8 +151,10 @@ #ifdef EARLY_BETA_OR_EARLIER #define IS_EARLY_BETA_OR_EARLIER true +#define IS_NOT_EARLY_BETA_OR_EARLIER false #else #define IS_EARLY_BETA_OR_EARLIER false +#define IS_NOT_EARLY_BETA_OR_EARLIER true #endif #ifdef ANDROID @@ -1447,6 +1449,13 @@ value: false mirror: always +# If set this to true, `Document.execCommand` may be performed nestedly. +# Otherwise, nested calls just return false. +- name: dom.document.exec_command.nested_calls_allowed + type: bool + value: @IS_NOT_EARLY_BETA_OR_EARLIER@ + mirror: always + - name: dom.enable_window_print type: bool value: @IS_NOT_ANDROID@