From b2fd51c0515c4291489f55a7daf023c3e41a9a93 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 24 Mar 2021 01:55:08 +0000 Subject: [PATCH] Bug 1220696 - part 4: Make `Document` consider whether the target is editable or not-editable with target editor r=smaug Currently, `Document` checks it only with whether the document is editable or not. Only with this check, `execCommand` and the other related methods work only when there is `contenteditable`. Therefore, this patch makes it to check whether the target is editable or not with target editor. Differential Revision: https://phabricator.services.mozilla.com/D108570 --- dom/base/Document.cpp | 67 +++--- dom/base/Document.h | 6 +- ...ommand-with-text-editor.tentative.html.ini | 210 ------------------ 3 files changed, 33 insertions(+), 250 deletions(-) diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp index e189222f32ee..9b3865154165 100644 --- a/dom/base/Document.cpp +++ b/dom/base/Document.cpp @@ -5103,6 +5103,21 @@ TextEditor* Document::AutoEditorCommandTarget::GetTargetEditor() const { return nullptr; } +bool Document::AutoEditorCommandTarget::IsEditable(Document* aDocument) const { + if (RefPtr doc = aDocument->GetInProcessParentDocument()) { + // Make sure frames are up to date, since that can affect whether + // we're editable. + doc->FlushPendingNotifications(FlushType::Frames); + } + TextEditor* targetEditor = GetTargetEditor(); + if (targetEditor && targetEditor->IsTextEditor()) { + // FYI: When `disabled` attribute is set, `TextEditor` treats it as + // "readonly" too. + return !targetEditor->IsReadonly(); + } + return aDocument->IsEditingOn(); +} + bool Document::AutoEditorCommandTarget::IsCommandEnabled() const { TextEditor* targetEditor = GetTargetEditor(); if (!targetEditor) { @@ -5187,11 +5202,6 @@ bool Document::ExecCommand(const nsAString& aHTMLCommandName, bool aShowUI, return false; } - // if editing is not on, bail - if (commandData.IsAvailableOnlyWhenEditable() && !IsEditingOnAfterFlush()) { - return false; - } - if (commandData.mCommand == Command::GetHTML) { return false; } @@ -5217,6 +5227,11 @@ bool Document::ExecCommand(const nsAString& aHTMLCommandName, bool aShowUI, // by order of controllers in `nsCommandManager::GetControllerForCommand()`. RefPtr presContext = GetPresContext(); AutoEditorCommandTarget editCommandTarget(presContext, commandData); + if (commandData.IsAvailableOnlyWhenEditable() && + !editCommandTarget.IsEditable(this)) { + return false; + } + if (editCommandTarget.DoNothing()) { return false; } @@ -5359,13 +5374,12 @@ bool Document::QueryCommandEnabled(const nsAString& aHTMLCommandName, return false; } - // if editing is not on, bail - if (!IsEditingOnAfterFlush()) { + RefPtr presContext = GetPresContext(); + AutoEditorCommandTarget editCommandTarget(presContext, commandData); + if (!editCommandTarget.IsEditable(this)) { return false; } - RefPtr presContext = GetPresContext(); - AutoEditorCommandTarget editCommandTarget(presContext, commandData); if (editCommandTarget.IsEditor()) { return editCommandTarget.IsCommandEnabled(); } @@ -5399,13 +5413,11 @@ bool Document::QueryCommandIndeterm(const nsAString& aHTMLCommandName, return false; } - // if editing is not on, bail - if (!IsEditingOnAfterFlush()) { - return false; - } - RefPtr presContext = GetPresContext(); AutoEditorCommandTarget editCommandTarget(presContext, commandData); + if (!editCommandTarget.IsEditable(this)) { + return false; + } RefPtr params = new nsCommandParams(); if (editCommandTarget.IsEditor()) { if (NS_FAILED(editCommandTarget.GetCommandStateParams(*params))) { @@ -5449,11 +5461,6 @@ bool Document::QueryCommandState(const nsAString& aHTMLCommandName, return false; } - // if editing is not on, bail - if (!IsEditingOnAfterFlush()) { - return false; - } - if (aHTMLCommandName.LowerCaseEqualsLiteral("usecss")) { // Per spec, state is supported for styleWithCSS but not useCSS, so we just // return false always. @@ -5462,6 +5469,9 @@ bool Document::QueryCommandState(const nsAString& aHTMLCommandName, RefPtr presContext = GetPresContext(); AutoEditorCommandTarget editCommandTarget(presContext, commandData); + if (!editCommandTarget.IsEditable(this)) { + return false; + } RefPtr params = new nsCommandParams(); if (editCommandTarget.IsEditor()) { if (NS_FAILED(editCommandTarget.GetCommandStateParams(*params))) { @@ -5588,13 +5598,11 @@ void Document::QueryCommandValue(const nsAString& aHTMLCommandName, return; } - // if editing is not on, bail - if (!IsEditingOnAfterFlush()) { - return; - } - RefPtr presContext = GetPresContext(); AutoEditorCommandTarget editCommandTarget(presContext, commandData); + if (!editCommandTarget.IsEditable(this)) { + return; + } RefPtr params = new nsCommandParams(); // FYI: Only GetHTML command is not implemented by editor. Use window's // command table instead. @@ -5655,17 +5663,6 @@ void Document::QueryCommandValue(const nsAString& aHTMLCommandName, CopyUTF8toUTF16(result, aValue); } -bool Document::IsEditingOnAfterFlush() { - RefPtr doc = GetInProcessParentDocument(); - if (doc) { - // Make sure frames are up to date, since that can affect whether - // we're editable. - doc->FlushPendingNotifications(FlushType::Frames); - } - - return IsEditingOn(); -} - void Document::MaybeEditingStateChanged() { if (!mPendingMaybeEditingStateChanged && mMayStartLayout && mUpdateNestLevel == 0 && (mContentEditableCount > 0) != IsEditingOn()) { diff --git a/dom/base/Document.h b/dom/base/Document.h index e900b12673ca..fd01e15c2b7d 100644 --- a/dom/base/Document.h +++ b/dom/base/Document.h @@ -1578,11 +1578,6 @@ class Document : public nsINode, */ void DisconnectNodeTree(); - /** - * Like IsEditingOn(), but will flush as needed first. - */ - bool IsEditingOnAfterFlush(); - /** * MaybeDispatchCheckKeyPressEventModelEvent() dispatches * "CheckKeyPressEventModel" event to check whether we should dispatch @@ -4197,6 +4192,7 @@ class Document : public nsINode, delete; bool DoNothing() const { return mDoNothing; } + MOZ_CAN_RUN_SCRIPT bool IsEditable(Document* aDocument) const; bool IsEditor() const { MOZ_ASSERT_IF(mEditorCommand, mActiveEditor || mHTMLEditor); return !!mEditorCommand; diff --git a/testing/web-platform/meta/editing/other/exec-command-with-text-editor.tentative.html.ini b/testing/web-platform/meta/editing/other/exec-command-with-text-editor.tentative.html.ini index 5fff9970c100..fedf208d8852 100644 --- a/testing/web-platform/meta/editing/other/exec-command-with-text-editor.tentative.html.ini +++ b/testing/web-platform/meta/editing/other/exec-command-with-text-editor.tentative.html.ini @@ -35,102 +35,12 @@ [In , execCommand("paste", false, null), a[\]c): The command should be enabled] expected: FAIL - [In , execCommand("delete", false, null), ab[\]c): The command should be enabled] - expected: FAIL - - [In , execCommand("delete", false, null), ab[\]c): execCommand() should return true] - expected: FAIL - - [In , execCommand("delete", false, null), ab[\]c): .value should be "a[\]c"] - expected: FAIL - - [In , execCommand("delete", false, null), ab[\]c): input.inputType should be deleteContentBackward] - expected: FAIL - - [In , execCommand("delete", false, null), ab[\]c): input.target should be [object HTMLInputElement\]] - expected: FAIL - - [In , execCommand("delete", false, null), a[b\]c): The command should be enabled] - expected: FAIL - - [In , execCommand("delete", false, null), a[b\]c): execCommand() should return true] - expected: FAIL - - [In , execCommand("delete", false, null), a[b\]c): .value should be "a[\]c"] - expected: FAIL - - [In , execCommand("delete", false, null), a[b\]c): input.inputType should be deleteContentBackward] - expected: FAIL - - [In , execCommand("delete", false, null), a[b\]c): input.target should be [object HTMLInputElement\]] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[b\]c): The command should be enabled] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[b\]c): execCommand() should return true] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[b\]c): .value should be "a[\]c"] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[b\]c): input.inputType should be deleteContentForward] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[b\]c): input.target should be [object HTMLInputElement\]] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[\]bc): The command should be enabled] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[\]bc): execCommand() should return true] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[\]bc): .value should be "a[\]c"] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[\]bc): input.inputType should be deleteContentForward] - expected: FAIL - - [In , execCommand("forwarddelete", false, null), a[\]bc): input.target should be [object HTMLInputElement\]] - expected: FAIL - - [In , execCommand("selectall", false, null), a[b\]c): The command should be enabled] - expected: FAIL - - [In , execCommand("selectall", false, null), a[b\]c): execCommand() should return true] - expected: FAIL - - [In , execCommand("selectall", false, null), a[b\]c): .value should be "[abc\]"] - expected: FAIL - [In , execCommand("undo", false, null), a[b\]c): The command should be enabled] expected: FAIL - [In , execCommand("undo", false, null), a[b\]c): execCommand() should return true] - expected: FAIL - - [In , execCommand("undo", false, null), a[b\]c): input.inputType should be historyUndo] - expected: FAIL - - [In , execCommand("undo", false, null), a[b\]c): input.target should be [object HTMLInputElement\]] - expected: FAIL - [In , execCommand("redo", false, null), a[b\]c): The command should be enabled] expected: FAIL - [In , execCommand("redo", false, null), a[b\]c): execCommand() should return true] - expected: FAIL - - [In , execCommand("redo", false, null), a[b\]c): .value should be "a[\]c"] - expected: FAIL - - [In , execCommand("redo", false, null), a[b\]c): input.inputType should be historyRedo] - expected: FAIL - - [In , execCommand("redo", false, null), a[b\]c): input.target should be [object HTMLInputElement\]] - expected: FAIL - [In , execCommand("inserthtml", false, inserted), a[b\]c): The command should be enabled] expected: FAIL @@ -146,21 +56,6 @@ [In , execCommand("inserthtml", false, inserted), a[b\]c): input.target should be [object HTMLInputElement\]] expected: FAIL - [In , execCommand("inserttext", false, **inserted**), a[b\]c): The command should be enabled] - expected: FAIL - - [In , execCommand("inserttext", false, **inserted**), a[b\]c): execCommand() should return true] - expected: FAIL - - [In , execCommand("inserttext", false, **inserted**), a[b\]c): .value should be "a**inserted**[\]c"] - expected: FAIL - - [In , execCommand("inserttext", false, **inserted**), a[b\]c): input.inputType should be insertText] - expected: FAIL - - [In , execCommand("inserttext", false, **inserted**), a[b\]c): input.target should be [object HTMLInputElement\]] - expected: FAIL - [In , execCommand("contentReadOnly", false, true), a[b\]c): The command should be enabled] expected: FAIL @@ -227,102 +122,12 @@ [In