Editing: Support |EditCommandSource| in |CompositeEditCommand| |
|||||||||
Issue descriptionThe suggestion came from yosin@'s review in https://crrev.com/2558643003/#msg26. We want to pass command source from |Editor::Command| down to |EditCommand| (or |CompositeEditCommand| which I believe is better), so we can then move |InputEvent| dispatching logic into |CompositeEditCommand::willApply()| as suggested in issue 670035. ---- Re-open as we want to start over according to https://codereview.chromium.org/2627103003/#msg12: ``` We should start over == reverting following patch first: - 1dfe560 [InputEvent] Remove unused |inputType()| from |UndoStep| and |EditCommandComposition| by chongz · 3 weeks ago - c659648 [Editing] Introduce |EditCommandComposition::willUn/Reapply()| in prepare for 'beforeinput' (2/3) by chongz · 3 weeks ago - 4d0f52c [Editing] Introduce |CompositeEditCommand::willApplyEditing()| in prepare for 'beforeinput' (1/3) by chongz · 3 weeks ago - acce62c [EditCommandSource] Pass source to |CompositEditCommand| and |TypingCommand| (3/3) by chongz · 4 weeks ago - 17c7368 [EditCommandSource] Rename and move |EditorCommandSource| to "CompositeEditCommand.h" (1/3) by chongz · 4 weeks ago Then 1. Introduce |enum class EditCommandSouce| and CompoisiteCommand ctor takes it as default parameter = kInternal. 2. Make ApplyBlockElementCommand aware of EditCommandSource 3. Make ApplyStyleCommand aware of EditCommandSource 4. ... N. Make UlinkCommand aware of EditCommandSouce N+1. Implement dispatch "beforeinput" to with EditCommandSource support. In this way, each patch is *small* and easy to review. You don't need to wait for review for reverting, just use TBR=yosin@chromium.org is fine. ``` We also need to revert * https://codereview.chromium.org/2594313002 Update layout after TypingCommand::willAddTypingToOpenCommand * https://codereview.chromium.org/2576903002/ [EditCommandSource] Pass source from |Editor::Command| to |Editor::apply*()/insert*()/...| (2/3)
,
Dec 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17c7368db5e14c05733b204bbf79e73998810d4b commit 17c7368db5e14c05733b204bbf79e73998810d4b Author: chongz <chongz@chromium.org> Date: Fri Dec 16 01:50:50 2016 [EditCommandSource] Rename and move |EditorCommandSource| to "CompositeEditCommand.h" (1/3) This is the sub-patch (1/3) of supporting |EditCommandSource| in |CompositeEditCommand| (https://crrev.com/2574793002). This CL: 1. Renamed |EditorCommandSource| => |EditCommandSource| 2. Changed type |enum| => |enum class| 3. Moved from "Editor.h" => "CompositeEditCommand.h" This CL shouldn't have any behavior change. BUG=673789 Review-Url: https://codereview.chromium.org/2578753002 Cr-Commit-Position: refs/heads/master@{#438979} [modify] https://crrev.com/17c7368db5e14c05733b204bbf79e73998810d4b/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/17c7368db5e14c05733b204bbf79e73998810d4b/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/17c7368db5e14c05733b204bbf79e73998810d4b/third_party/WebKit/Source/core/editing/Editor.h [modify] https://crrev.com/17c7368db5e14c05733b204bbf79e73998810d4b/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/17c7368db5e14c05733b204bbf79e73998810d4b/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp
,
Dec 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f6fddd7fab5cf706023fc4f2edbdfa4fdd93d56f commit f6fddd7fab5cf706023fc4f2edbdfa4fdd93d56f Author: chongz <chongz@chromium.org> Date: Fri Dec 16 03:46:39 2016 [EditCommandSource] Pass source from |Editor::Command| to |Editor::apply*()/insert*()/...| (2/3) This is the sub-patch (2/3) of supporting |EditCommandSource| in |CompositeEditCommand| (https://crrev.com/2574793002). This CL: 1. Added method |Editor::replaceSelectionForSpellChecker()| for |SpellChecker::replaceMisspelledRange()| to hide editor details; 2. Passed command source from |Editor::Command| to various methods in |Editor|, such as |performDelete()|, |removeFormattingAndStyle()|, |applyStyle()|, |replaceSelectionWithText()| etc. Note: It's OK to add extra |EditCommandSource| parameter to those methods because they are going to be moved to "EditorCommand.cpp" (according to the editing refactor doc), which means they should share the similar pattern as other |static execute*()| methods. This CL shouldn't have any behavior change. Next CL will pass command source down to |CompositeEditCommand| and |TypingCommand|. Editing Refactor doc: https://docs.google.com/a/chromium.org/document/d/1i-f5ByGHIvTcvy2UEAvOtBK14BNMFTB8ms9uedeArbw/edit?usp=sharing BUG=673789 Review-Url: https://codereview.chromium.org/2576903002 Cr-Commit-Position: refs/heads/master@{#439010} [modify] https://crrev.com/f6fddd7fab5cf706023fc4f2edbdfa4fdd93d56f/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/f6fddd7fab5cf706023fc4f2edbdfa4fdd93d56f/third_party/WebKit/Source/core/editing/Editor.h [modify] https://crrev.com/f6fddd7fab5cf706023fc4f2edbdfa4fdd93d56f/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp [modify] https://crrev.com/f6fddd7fab5cf706023fc4f2edbdfa4fdd93d56f/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp [modify] https://crrev.com/f6fddd7fab5cf706023fc4f2edbdfa4fdd93d56f/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
,
Dec 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/acce62cb77947bf2f14d6c7ac74219d515a94ffb commit acce62cb77947bf2f14d6c7ac74219d515a94ffb Author: chongz <chongz@chromium.org> Date: Fri Dec 16 17:11:41 2016 [EditCommandSource] Pass source to |CompositEditCommand| and |TypingCommand| (3/3) This is the last sub-patch (3/3) of supporting |EditCommandSource| in |CompositeEditCommand| (https://crrev.com/2574793002). Instead of adding |EditCommandSource| to constructor, this CL: 1. Pass source to |CompositeEditCommand::apply(EditCommandSource)| 2. Pass source to various static methods of |TypingCommand| We don't have to store command source as attribute because we only need it to fire 'beforeinput' in the above locations. This approach also helped avoiding modifying all the |CompositeEditCommand| constructors. This CL shouldn't have any behavior change. To help reviewing, here is a simple doc describing the following plans: https://docs.google.com/a/chromium.org/document/d/1M9bQaLKdRpR1qJ9i7zZKN-hKwd4ZkgJs5GPde2ZsDy0/edit?usp=sharing BUG=673789 Review-Url: https://codereview.chromium.org/2579253002 Cr-Commit-Position: refs/heads/master@{#439130} [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/InputMethodController.cpp [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommandTest.cpp [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommandTest.cpp [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/InsertListCommandTest.cpp [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/TypingCommand.h [modify] https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb/third_party/WebKit/Source/core/editing/commands/TypingCommandTest.cpp
,
Dec 16 2016
,
Dec 16 2016
,
Jan 13 2017
Re-open as we want to start over according to https://codereview.chromium.org/2627103003/#msg12: ``` We should start over == reverting following patch first: - 1dfe560 [InputEvent] Remove unused |inputType()| from |UndoStep| and |EditCommandComposition| by chongz · 3 weeks ago - c659648 [Editing] Introduce |EditCommandComposition::willUn/Reapply()| in prepare for 'beforeinput' (2/3) by chongz · 3 weeks ago - 4d0f52c [Editing] Introduce |CompositeEditCommand::willApplyEditing()| in prepare for 'beforeinput' (1/3) by chongz · 3 weeks ago - acce62c [EditCommandSource] Pass source to |CompositEditCommand| and |TypingCommand| (3/3) by chongz · 4 weeks ago - 17c7368 [EditCommandSource] Rename and move |EditorCommandSource| to "CompositeEditCommand.h" (1/3) by chongz · 4 weeks ago Then 1. Introduce |enum class EditCommandSouce| and CompoisiteCommand ctor takes it as default parameter = kInternal. 2. Make ApplyBlockElementCommand aware of EditCommandSource 3. Make ApplyStyleCommand aware of EditCommandSource 4. ... N. Make UlinkCommand aware of EditCommandSouce N+1. Implement dispatch "beforeinput" to with EditCommandSource support. In this way, each patch is *small* and easy to review. You don't need to wait for review for reverting, just use TBR=yosin@chromium.org is fine. ```
,
Jan 13 2017
,
Jan 16 2017
,
Jan 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e2dafbf4760e826f18b0c16b3976a667fcacf8a0 commit e2dafbf4760e826f18b0c16b3976a667fcacf8a0 Author: chongz <chongz@chromium.org> Date: Mon Jan 16 23:28:35 2017 Revert of [EditCommandSource] Pass source to |CompositEditCommand| and |TypingCommand| (3/3) (patchset #1 id:20001 of https://codereview.chromium.org/2579253002/ ) Reason for revert: We want to revert all CLs and start over the refactor process, see: https://crbug.com/673789. Original issue's description: > [EditCommandSource] Pass source to |CompositEditCommand| and |TypingCommand| (3/3) > > This is the last sub-patch (3/3) of supporting |EditCommandSource| in > |CompositeEditCommand| (https://crrev.com/2574793002). > > Instead of adding |EditCommandSource| to constructor, this CL: > 1. Pass source to |CompositeEditCommand::apply(EditCommandSource)| > 2. Pass source to various static methods of |TypingCommand| > > We don't have to store command source as attribute because we only need it to fire > 'beforeinput' in the above locations. > This approach also helped avoiding modifying all the |CompositeEditCommand| > constructors. > > This CL shouldn't have any behavior change. > > To help reviewing, here is a simple doc describing the following plans: > https://docs.google.com/a/chromium.org/document/d/1M9bQaLKdRpR1qJ9i7zZKN-hKwd4ZkgJs5GPde2ZsDy0/edit?usp=sharing > > BUG=673789 > > Committed: https://crrev.com/acce62cb77947bf2f14d6c7ac74219d515a94ffb > Cr-Commit-Position: refs/heads/master@{#439130} TBR=tkent@chromium.org,xiaochengh@chromium.org,yosin@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=673789 Review-Url: https://codereview.chromium.org/2636043002 Cr-Commit-Position: refs/heads/master@{#443960} [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/InputMethodController.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommandTest.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommandTest.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommandTest.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/InsertListCommandTest.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/TypingCommand.h [modify] https://crrev.com/e2dafbf4760e826f18b0c16b3976a667fcacf8a0/third_party/WebKit/Source/core/editing/commands/TypingCommandTest.cpp
,
Jan 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5d43e0f5fb5702f1873a4e9e94ea0f418ded2406 commit 5d43e0f5fb5702f1873a4e9e94ea0f418ded2406 Author: chongz <chongz@chromium.org> Date: Tue Jan 17 17:48:49 2017 Revert of [EditCommandSource] Pass source from |Editor::Command| to |Editor::apply*()/insert*()/...| (2/3) (patchset #2 id:40001 of https://codereview.chromium.org/2576903002/ ) Reason for revert: We want to revert all CLs and start over the refactor process, see: https://crbug.com/673789. Original issue's description: > [EditCommandSource] Pass source from |Editor::Command| to |Editor::apply*()/insert*()/...| (2/3) > > This is the sub-patch (2/3) of supporting |EditCommandSource| in > |CompositeEditCommand| (https://crrev.com/2574793002). > > This CL: > 1. Added method |Editor::replaceSelectionForSpellChecker()| for > |SpellChecker::replaceMisspelledRange()| to hide editor details; > 2. Passed command source from |Editor::Command| to various methods in |Editor|, > such as |performDelete()|, |removeFormattingAndStyle()|, |applyStyle()|, > |replaceSelectionWithText()| etc. > > Note: > It's OK to add extra |EditCommandSource| parameter to those methods because > they are going to be moved to "EditorCommand.cpp" (according to the editing > refactor doc), which means they should share the similar pattern as other > |static execute*()| methods. > > This CL shouldn't have any behavior change. > > Next CL will pass command source down to |CompositeEditCommand| and > |TypingCommand|. > > Editing Refactor doc: > https://docs.google.com/a/chromium.org/document/d/1i-f5ByGHIvTcvy2UEAvOtBK14BNMFTB8ms9uedeArbw/edit?usp=sharing > > BUG=673789 > > Committed: https://crrev.com/f6fddd7fab5cf706023fc4f2edbdfa4fdd93d56f > Cr-Commit-Position: refs/heads/master@{#439010} TBR=tkent@chromium.org,xiaochengh@chromium.org,yosin@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=673789 Review-Url: https://codereview.chromium.org/2635963004 Cr-Commit-Position: refs/heads/master@{#444085} [modify] https://crrev.com/5d43e0f5fb5702f1873a4e9e94ea0f418ded2406/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/5d43e0f5fb5702f1873a4e9e94ea0f418ded2406/third_party/WebKit/Source/core/editing/Editor.h [modify] https://crrev.com/5d43e0f5fb5702f1873a4e9e94ea0f418ded2406/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp [modify] https://crrev.com/5d43e0f5fb5702f1873a4e9e94ea0f418ded2406/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp [modify] https://crrev.com/5d43e0f5fb5702f1873a4e9e94ea0f418ded2406/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
,
Jan 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b45dc22297adb1eba7518c2731ec1de87513e99 commit 5b45dc22297adb1eba7518c2731ec1de87513e99 Author: chongz <chongz@chromium.org> Date: Wed Jan 18 18:00:56 2017 Revert of [EditCommandSource] Rename and move |EditorCommandSource| to "CompositeEditCommand.h" (1/3) (patchset #2 id:40001 of https://codereview.chromium.org/2578753002/ ) Reason for revert: We want to revert all CLs and start over the refactor process, see: https://crbug.com/673789. Original issue's description: > [EditCommandSource] Rename and move |EditorCommandSource| to "CompositeEditCommand.h" (1/3) > > This is the sub-patch (1/3) of supporting |EditCommandSource| in > |CompositeEditCommand| (https://crrev.com/2574793002). > > This CL: > 1. Renamed |EditorCommandSource| => |EditCommandSource| > 2. Changed type |enum| => |enum class| > 3. Moved from "Editor.h" => "CompositeEditCommand.h" > > This CL shouldn't have any behavior change. > > BUG=673789 > > Committed: https://crrev.com/17c7368db5e14c05733b204bbf79e73998810d4b > Cr-Commit-Position: refs/heads/master@{#438979} TBR=tkent@chromium.org,xiaochengh@chromium.org,yosin@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=673789 Review-Url: https://codereview.chromium.org/2636963003 Cr-Commit-Position: refs/heads/master@{#444417} [modify] https://crrev.com/5b45dc22297adb1eba7518c2731ec1de87513e99/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/5b45dc22297adb1eba7518c2731ec1de87513e99/third_party/WebKit/Source/core/editing/Editor.h [modify] https://crrev.com/5b45dc22297adb1eba7518c2731ec1de87513e99/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/5b45dc22297adb1eba7518c2731ec1de87513e99/third_party/WebKit/Source/core/editing/commands/DocumentExecCommand.cpp [modify] https://crrev.com/5b45dc22297adb1eba7518c2731ec1de87513e99/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp
,
Nov 13 2017
,
Nov 14
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 19
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by bugdroid1@chromium.org
, Dec 15 2016