Editing commands should not store VisibleSelections |
|||||
Issue descriptionEditCommand and EditCommandComposition store VisibleSelection as starting and ending selections. However, these VisibleSelections are stored through DOM and style changes, during which they may no longer represent a range of canonical positions. Therefore, EditCommand and EditCommandComposition should store something else that still remaining valid meaningful after DOM and style changes. This also allows us to reduce the number of layout update calls in editing/commands.
,
Jul 25 2017
,
Aug 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/693b3f0325abdcde6cf63511dd488c8d62254dea commit 693b3f0325abdcde6cf63511dd488c8d62254dea Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Wed Aug 16 07:56:11 2017 Use SelectionForUndoStep in ApplyBlockElementCommand ApplyBlockElementCommand uses SelectionInDOMTree version of SetEndingSelection, which do unnecessary layout updates and canonicalization. We should use SelectionForUndoStep version while doing SetEndingSelection, as EndingSelection expects to be maintained through out the DOM and style changes. BUG= 657256 Change-Id: I170f52757162a4ba17e242a965891e4978d2a76f Reviewed-on: https://chromium-review.googlesource.com/612974 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Cr-Commit-Position: refs/heads/master@{#494720} [modify] https://crrev.com/693b3f0325abdcde6cf63511dd488c8d62254dea/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp
,
Aug 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e33faceb080c6ab5c3bed2acd71782da5628497 commit 4e33faceb080c6ab5c3bed2acd71782da5628497 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Wed Aug 16 11:32:11 2017 Use SelectionForUndoStep in CompositeEditCommand::ApplyCommandToComposite Currently CompositeEditCommand::ApplyCommandToComposite uses VisibleSelection, to reduce the usage of VisibleSelection and also prepare for movement of is_directional_ from SelectionController to FrameSelection we will use SelectionForUndoStep instead of VisibleSelection BUG= 740804 , 657256 Change-Id: I17bb7d0790bd1dfd023d14a0ff8610cdee028e33 Reviewed-on: https://chromium-review.googlesource.com/605416 Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#494748} [modify] https://crrev.com/4e33faceb080c6ab5c3bed2acd71782da5628497/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/4e33faceb080c6ab5c3bed2acd71782da5628497/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/4e33faceb080c6ab5c3bed2acd71782da5628497/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.cpp [modify] https://crrev.com/4e33faceb080c6ab5c3bed2acd71782da5628497/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.h [modify] https://crrev.com/4e33faceb080c6ab5c3bed2acd71782da5628497/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp
,
Aug 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6d31ab9fdcfc621485c67d0efd611c9430ab1742 commit 6d31ab9fdcfc621485c67d0efd611c9430ab1742 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Thu Aug 17 02:04:23 2017 Use SelectionForUndoStep in InsertListCommand Currently SetEndingSelection uses SelectionInDOMTree for setting ending selection, this does layout update and unnecessary canononicalization to create VisibleSelection which is not needed, because SetEndingSelection is stored through the DOM and style changes. Therefore we will use SelectionForUndoStep for setting the ending selection, as InsertListCommand do not require any canonicalized data or layout update to store the ending selection. BUG= 657256 Change-Id: If06f661ec2b170f808439a55ec169e3d20cefd89 Reviewed-on: https://chromium-review.googlesource.com/616512 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#495050} [modify] https://crrev.com/6d31ab9fdcfc621485c67d0efd611c9430ab1742/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp
,
Aug 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b16aed1566bfb2f42cad30f3d58c8c300bbd6ac5 commit b16aed1566bfb2f42cad30f3d58c8c300bbd6ac5 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Thu Aug 17 04:01:00 2017 Use SelectionForUndoStep in InsertParagraphSeparatorCommand Currently SetEndingSelection uses SelectionInDOMTree for setting ending selection, this does layout update and unnecessary canononicalization to create VisibleSelection which is not needed, because SetEndingSelection is stored through the DOM and style changes. Therefore we will use SelectionForUndoStep for setting the ending selection, as InsertParagraphSeparatorCommand do not require any canonicalized data or layout update to store the ending selection. BUG= 657256 Change-Id: I42d1c45249bb91a44e414bc08d9352218f965e5c Reviewed-on: https://chromium-review.googlesource.com/616514 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#495065} [modify] https://crrev.com/b16aed1566bfb2f42cad30f3d58c8c300bbd6ac5/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp
,
Aug 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8e635f2266955c40e40991a77d41ea0ec8c1abd commit b8e635f2266955c40e40991a77d41ea0ec8c1abd Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Thu Aug 17 04:23:07 2017 Use SelectionForUndoStep in InsertLineBreakCommand InsertLineBreakCommand uses SelectionInDOMTree version of SetEndingSelection, which do unnecessary layout updates and canonicalization. We should use SelectionForUndoStep version while doing SetEndingSelection, as EndingSelection expects to be maintained through out the DOM and style changes. BUG= 657256 Change-Id: Ie2a6b485541283302253a3ec49638316555ed54d Reviewed-on: https://chromium-review.googlesource.com/616492 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#495070} [modify] https://crrev.com/b8e635f2266955c40e40991a77d41ea0ec8c1abd/third_party/WebKit/Source/core/editing/commands/InsertLineBreakCommand.cpp
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ed23732eddae7a8f46d7e013a0743401872e7337 commit ed23732eddae7a8f46d7e013a0743401872e7337 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Fri Aug 18 06:14:47 2017 Use SelectionForUndoStep in ApplyStyleCommand Currently SetEndingSelection uses SelectionInDOMTree for setting ending selection, this does layout update and unnecessary canononicalization to create VisibleSelection which is not needed, because SetEndingSelection is stored through the DOM and style changes. As ApplyStyleCommand do not require any canonicalized data or layout update to store the ending selection, therefore we will use SelectionForUndoStep for setting the ending selection. BUG= 657256 Change-Id: Ic9fc164f7a54dd143be8e0ba1a7a5f90465ed125 Reviewed-on: https://chromium-review.googlesource.com/618213 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#495477} [modify] https://crrev.com/ed23732eddae7a8f46d7e013a0743401872e7337/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0bfd0d23c979b676713ea871a03efca2e6c2cdc3 commit 0bfd0d23c979b676713ea871a03efca2e6c2cdc3 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Fri Aug 18 06:16:28 2017 ApplyStyleCommand position should use SelectionForUndoStep ApplyStyleCommand calculates start and end position from VisibleSelection, which is canonicalized value. As we are making EditorCommands to use non canonicalized value, so we should use SelectionForUndoStep stored position for calculating start and end position. On using non canonicalized value for setting ending selection for InsertTextCommand, InsertTextCommandTest failed as we are using VS to get the positions. BUG= 657256 Change-Id: I9098456147c731192d63fcb09646556950435c5c Reviewed-on: https://chromium-review.googlesource.com/618211 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#495479} [modify] https://crrev.com/0bfd0d23c979b676713ea871a03efca2e6c2cdc3/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/661f4cb81c794b52773b116de0b25775111c04bf commit 661f4cb81c794b52773b116de0b25775111c04bf Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Fri Aug 18 06:21:52 2017 Use SelectionForUndoStep in BreakBlockquoteCommand Currently SetEndingSelection uses SelectionInDOMTree for setting ending selection, this does layout update and unnecessary canononicalization to create VisibleSelection which is not needed, because SetEndingSelection is stored through the DOM and style changes. As BreakBlockquoteCommand do not require any canonicalized data or layout update to store the ending selection, therefore we will use SelectionForUndoStep for setting the ending selection. BUG= 657256 Change-Id: I7c921ce63ec50f36b9dc63080af95a945050c140 Reviewed-on: https://chromium-review.googlesource.com/618435 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#495482} [modify] https://crrev.com/661f4cb81c794b52773b116de0b25775111c04bf/third_party/WebKit/Source/core/editing/commands/BreakBlockquoteCommand.cpp
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ed629b3006a46ea050d9e3a2faab711dbb2525f1 commit ed629b3006a46ea050d9e3a2faab711dbb2525f1 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Fri Aug 18 06:22:43 2017 Use SelectionForUndoStep in CreateLinkCommand Currently SetEndingSelection uses SelectionInDOMTree for setting ending selection, this does layout update and unnecessary canononicalization to create VisibleSelection which is not needed, because SetEndingSelection is stored through the DOM and style changes. As CreateLinkCommand do not require any canonicalized data or layout update to store the ending selection, therefore we will use SelectionForUndoStep for setting the ending selection. BUG= 657256 Change-Id: Ie4daff4846167234bb9b5ca6fdd6ccf2cbb0703a Reviewed-on: https://chromium-review.googlesource.com/618827 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#495483} [modify] https://crrev.com/ed629b3006a46ea050d9e3a2faab711dbb2525f1/third_party/WebKit/Source/core/editing/commands/CreateLinkCommand.cpp
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d6ddd99bd9e3e2318a5d975f64a6cd9a978c22c commit 1d6ddd99bd9e3e2318a5d975f64a6cd9a978c22c Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Fri Aug 18 06:26:07 2017 Use SelectionForUndoStep in IndentOutdentCommand Currently SetEndingSelection uses SelectionInDOMTree for setting ending selection, this does layout update and unnecessary canononicalization to create VisibleSelection which is not needed, because SetEndingSelection is stored through the DOM and style changes. As IndentOutdentCommand do not require any canonicalized data or layout update to store the ending selection, therefore we will use SelectionForUndoStep for setting the ending selection. BUG= 657256 Change-Id: I74cfc4baaa1069a66c84a793ee58aa0a26e02113 Reviewed-on: https://chromium-review.googlesource.com/618828 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#495486} [modify] https://crrev.com/1d6ddd99bd9e3e2318a5d975f64a6cd9a978c22c/third_party/WebKit/Source/core/editing/commands/IndentOutdentCommand.cpp
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2c241cd1d3373dec695a0e9ea254c1f89b9251c4 commit 2c241cd1d3373dec695a0e9ea254c1f89b9251c4 Author: Henrik Kjellander <kjellander@chromium.org> Date: Fri Aug 18 09:45:35 2017 Revert "Use SelectionForUndoStep in IndentOutdentCommand" This reverts commit 1d6ddd99bd9e3e2318a5d975f64a6cd9a978c22c. Reason for revert: Speculative revert for failing webkit_layout_tests: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/49108 https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11/builds/22594 https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/60990 https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7/builds/54698 Original change's description: > Use SelectionForUndoStep in IndentOutdentCommand > > Currently SetEndingSelection uses SelectionInDOMTree > for setting ending selection, this does layout update > and unnecessary canononicalization to create VisibleSelection > which is not needed, because SetEndingSelection is stored through > the DOM and style changes. > As IndentOutdentCommand do not require any > canonicalized data or layout update to store the ending selection, > therefore we will use SelectionForUndoStep for setting the > ending selection. > > BUG= 657256 > > Change-Id: I74cfc4baaa1069a66c84a793ee58aa0a26e02113 > Reviewed-on: https://chromium-review.googlesource.com/618828 > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> > Cr-Commit-Position: refs/heads/master@{#495486} TBR=yosin@chromium.org,xiaochengh@chromium.org,tanvir.rizvi@samsung.com Change-Id: Ic45f4865cc09d652377be49be108ab2afa84fcdf No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 657256 Reviewed-on: https://chromium-review.googlesource.com/620686 Reviewed-by: Henrik Kjellander <kjellander@chromium.org> Commit-Queue: Henrik Kjellander <kjellander@chromium.org> Cr-Commit-Position: refs/heads/master@{#495511} [modify] https://crrev.com/2c241cd1d3373dec695a0e9ea254c1f89b9251c4/third_party/WebKit/Source/core/editing/commands/IndentOutdentCommand.cpp
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bbc699c71b4a90b4f3d1919d1d7d85534e6dfc96 commit bbc699c71b4a90b4f3d1919d1d7d85534e6dfc96 Author: Henrik Kjellander <kjellander@chromium.org> Date: Fri Aug 18 11:00:10 2017 Revert "Revert "Use SelectionForUndoStep in IndentOutdentCommand"" This reverts commit 2c241cd1d3373dec695a0e9ea254c1f89b9251c4. Relanding as it didn't affect the failing tests. Original change's description: > Revert "Use SelectionForUndoStep in IndentOutdentCommand" > > This reverts commit 1d6ddd99bd9e3e2318a5d975f64a6cd9a978c22c. > > Reason for revert: Speculative revert for failing webkit_layout_tests: > https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/49108 > https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11/builds/22594 > https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/60990 > https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7/builds/54698 > > Original change's description: > > Use SelectionForUndoStep in IndentOutdentCommand > > > > Currently SetEndingSelection uses SelectionInDOMTree > > for setting ending selection, this does layout update > > and unnecessary canononicalization to create VisibleSelection > > which is not needed, because SetEndingSelection is stored through > > the DOM and style changes. > > As IndentOutdentCommand do not require any > > canonicalized data or layout update to store the ending selection, > > therefore we will use SelectionForUndoStep for setting the > > ending selection. > > > > BUG= 657256 > > > > Change-Id: I74cfc4baaa1069a66c84a793ee58aa0a26e02113 > > Reviewed-on: https://chromium-review.googlesource.com/618828 > > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > > Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#495486} > > TBR=yosin@chromium.org,xiaochengh@chromium.org,tanvir.rizvi@samsung.com > > Change-Id: Ic45f4865cc09d652377be49be108ab2afa84fcdf > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 657256 > Reviewed-on: https://chromium-review.googlesource.com/620686 > Reviewed-by: Henrik Kjellander <kjellander@chromium.org> > Commit-Queue: Henrik Kjellander <kjellander@chromium.org> > Cr-Commit-Position: refs/heads/master@{#495511} TBR=kjellander@chromium.org,yosin@chromium.org,xiaochengh@chromium.org,tanvir.rizvi@samsung.com Change-Id: I3e88ccaacd5fc17505799fd380380931859a2768 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 657256 Reviewed-on: https://chromium-review.googlesource.com/620786 Reviewed-by: Henrik Kjellander <kjellander@chromium.org> Commit-Queue: Henrik Kjellander <kjellander@chromium.org> Cr-Commit-Position: refs/heads/master@{#495524} [modify] https://crrev.com/bbc699c71b4a90b4f3d1919d1d7d85534e6dfc96/third_party/WebKit/Source/core/editing/commands/IndentOutdentCommand.cpp
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/abc1dd4038f0831e3d1cd555997a6dc2484141b8 commit abc1dd4038f0831e3d1cd555997a6dc2484141b8 Author: Henrik Kjellander <kjellander@chromium.org> Date: Fri Aug 18 11:37:29 2017 Revert "ApplyStyleCommand position should use SelectionForUndoStep" This reverts commit 0bfd0d23c979b676713ea871a03efca2e6c2cdc3. Reason for revert: Speculative revert for failing webkit_layout_tests: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/49108 https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11/builds/22594 https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/60990 https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7/builds/54698 Potentially need to be combined with https://chromium-review.googlesource.com/c/620786 which I already relanded after seeing it didn't have any effect. Original change's description: > ApplyStyleCommand position should use SelectionForUndoStep > > ApplyStyleCommand calculates start and end position from > VisibleSelection, which is canonicalized value. > As we are making EditorCommands to use non canonicalized value, > so we should use SelectionForUndoStep stored position for > calculating start and end position. > > On using non canonicalized value for setting ending selection > for InsertTextCommand, InsertTextCommandTest failed > as we are using VS to get the positions. > > BUG= 657256 > > Change-Id: I9098456147c731192d63fcb09646556950435c5c > Reviewed-on: https://chromium-review.googlesource.com/618211 > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> > Cr-Commit-Position: refs/heads/master@{#495479} TBR=yosin@chromium.org,xiaochengh@chromium.org,tanvir.rizvi@samsung.com Change-Id: I54ca37d281d500c25d03a5f1d2e106690617574d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 657256 Reviewed-on: https://chromium-review.googlesource.com/620806 Reviewed-by: Henrik Kjellander <kjellander@chromium.org> Commit-Queue: Henrik Kjellander <kjellander@chromium.org> Cr-Commit-Position: refs/heads/master@{#495526} [modify] https://crrev.com/abc1dd4038f0831e3d1cd555997a6dc2484141b8/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ac4efceae812ffe3e4e96d34113a8bbfd5ea1bd7 commit ac4efceae812ffe3e4e96d34113a8bbfd5ea1bd7 Author: Tanvir Rizvi <tanvir.rizvi@samsung.com> Date: Fri Aug 18 18:10:17 2017 Revert "Use SelectionForUndoStep in ApplyStyleCommand" This reverts commit ed23732eddae7a8f46d7e013a0743401872e7337. Reason for revert: <INSERT REASONING HERE> Original change's description: > Use SelectionForUndoStep in ApplyStyleCommand > > Currently SetEndingSelection uses SelectionInDOMTree > for setting ending selection, this does layout update > and unnecessary canononicalization to create VisibleSelection > which is not needed, because SetEndingSelection is stored through > the DOM and style changes. > As ApplyStyleCommand do not require any > canonicalized data or layout update to store the ending selection, > therefore we will use SelectionForUndoStep for setting the > ending selection. > > BUG= 657256 > > Change-Id: Ic9fc164f7a54dd143be8e0ba1a7a5f90465ed125 > Reviewed-on: https://chromium-review.googlesource.com/618213 > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> > Cr-Commit-Position: refs/heads/master@{#495477} TBR=yosin@chromium.org,xiaochengh@chromium.org,tanvir.rizvi@samsung.com Change-Id: I906e4c8de4d6c6fbdb95b9d99c4efd82615656a7 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 657256 Reviewed-on: https://chromium-review.googlesource.com/620547 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#495619} [modify] https://crrev.com/ac4efceae812ffe3e4e96d34113a8bbfd5ea1bd7/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3325ec26ef63a6e13de5ff4e463c632c1afdf881 commit 3325ec26ef63a6e13de5ff4e463c632c1afdf881 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Mon Aug 21 12:02:40 2017 Get RootEditableElement from SelectionForUndoStep Currently RootEditableElement is calculated by doing EndingVisibleSelection, which do a layout and canonicalization which is not required. So we can use SelectionForUndoStep stored value to get the RootEditableElement BUG= 657256 Change-Id: I3221979236058dd6107154bdbaaf0c5f21cab7af Reviewed-on: https://chromium-review.googlesource.com/616376 Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#495917} [modify] https://crrev.com/3325ec26ef63a6e13de5ff4e463c632c1afdf881/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp [modify] https://crrev.com/3325ec26ef63a6e13de5ff4e463c632c1afdf881/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp [modify] https://crrev.com/3325ec26ef63a6e13de5ff4e463c632c1afdf881/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp [modify] https://crrev.com/3325ec26ef63a6e13de5ff4e463c632c1afdf881/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp [modify] https://crrev.com/3325ec26ef63a6e13de5ff4e463c632c1afdf881/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
,
Aug 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5fa1882fd5b00ff520b58f80ded1ae4079e79e06 commit 5fa1882fd5b00ff520b58f80ded1ae4079e79e06 Author: Tanvir Rizvi <tanvir.rizvi@samsung.com> Date: Tue Aug 22 06:23:58 2017 Revert "Revert "ApplyStyleCommand position should use SelectionForUndoStep"" This reverts commit abc1dd4038f0831e3d1cd555997a6dc2484141b8. Reason for revert: https://chromium-review.googlesource.com/c/618213 has been reverted which was causing the failures. This patch can now be re-landed. The patch was causing failures as we were avoiding canonicalization for setting the ending selection. Original change's description: > Revert "ApplyStyleCommand position should use SelectionForUndoStep" > > This reverts commit 0bfd0d23c979b676713ea871a03efca2e6c2cdc3. > > Reason for revert: Speculative revert for failing webkit_layout_tests: > https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/49108 > https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11/builds/22594 > https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/60990 > https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7/builds/54698 > > Potentially need to be combined with https://chromium-review.googlesource.com/c/620786 which I already relanded after > seeing it didn't have any effect. > > Original change's description: > > ApplyStyleCommand position should use SelectionForUndoStep > > > > ApplyStyleCommand calculates start and end position from > > VisibleSelection, which is canonicalized value. > > As we are making EditorCommands to use non canonicalized value, > > so we should use SelectionForUndoStep stored position for > > calculating start and end position. > > > > On using non canonicalized value for setting ending selection > > for InsertTextCommand, InsertTextCommandTest failed > > as we are using VS to get the positions. > > > > BUG= 657256 > > > > Change-Id: I9098456147c731192d63fcb09646556950435c5c > > Reviewed-on: https://chromium-review.googlesource.com/618211 > > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > > Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#495479} > > TBR=yosin@chromium.org,xiaochengh@chromium.org,tanvir.rizvi@samsung.com > > Change-Id: I54ca37d281d500c25d03a5f1d2e106690617574d > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 657256 > Reviewed-on: https://chromium-review.googlesource.com/620806 > Reviewed-by: Henrik Kjellander <kjellander@chromium.org> > Commit-Queue: Henrik Kjellander <kjellander@chromium.org> > Cr-Commit-Position: refs/heads/master@{#495526} TBR=kjellander@chromium.org,yosin@chromium.org,xiaochengh@chromium.org,tanvir.rizvi@samsung.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 657256 Change-Id: I0128138f4a94da1af736e4a62014fb08a570192d Reviewed-on: https://chromium-review.googlesource.com/623369 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Cr-Commit-Position: refs/heads/master@{#496231} [modify] https://crrev.com/5fa1882fd5b00ff520b58f80ded1ae4079e79e06/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp
,
Aug 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/256de0420b3d8cae855a4ce8f2f932bd92864435 commit 256de0420b3d8cae855a4ce8f2f932bd92864435 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Tue Aug 22 08:21:30 2017 Use SelectionForUndoStep in InsertTextCommand Currently SetEndingSelection uses SelectionInDOMTree for setting ending selection, this does layout update and unnecessary canononicalization to create VisibleSelection which is not needed, because SetEndingSelection is stored through the DOM and style changes. Therefore we will use SelectionForUndoStep for setting the ending selection, as InsertTextCommand do not require any canonicalized data or layout update to store the ending selection. BUG= 657256 Change-Id: I415aa53b67a03994e0b270fe98fc9bb182251630 Reviewed-on: https://chromium-review.googlesource.com/616410 Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#496249} [modify] https://crrev.com/256de0420b3d8cae855a4ce8f2f932bd92864435/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp
,
Aug 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28d5c5928d61e2aefc0fd959ced434ee784d87ef commit 28d5c5928d61e2aefc0fd959ced434ee784d87ef Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Tue Aug 29 09:52:35 2017 Use SelectionForUndoStep in ApplyStyleCommand ApplySyleCommand needs canonicalization to set start and end positions. Since we are trying to remove SetEndingSelection(SelectionInDOMTree&) from editor commands, this CL does the in-place changes to calculate the canonicalised values. Bug: 657256 Change-Id: I561b863fcb61259244c3f7349de74dcca4cd2fc1 Reviewed-on: https://chromium-review.googlesource.com/638190 Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#498065} [modify] https://crrev.com/28d5c5928d61e2aefc0fd959ced434ee784d87ef/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp
,
Aug 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/981ba78ebe26c6f9d75795406ccfd99bc56e8d0e commit 981ba78ebe26c6f9d75795406ccfd99bc56e8d0e Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Tue Aug 29 12:18:56 2017 Make SpellChecker not to use EndingVisibleSelection EndingVisibleSelection() call does canonicalization, which is not needed by SpellChecker, so we can use commands SelectionForUndoStep to get the required positions. Also we will make EndingVisibleSelection method of editing commands as protected method. Bug: 657256 Change-Id: Ic7cd9dca3bfc47df4e2e3942162a022f6ad0c70a Reviewed-on: https://chromium-review.googlesource.com/631762 Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#498079} [modify] https://crrev.com/981ba78ebe26c6f9d75795406ccfd99bc56e8d0e/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp
,
Sep 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0e199c8acaf3becd1f806f3d6f5b1927fab0740 commit f0e199c8acaf3becd1f806f3d6f5b1927fab0740 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Mon Sep 04 08:03:50 2017 Reduce usage of VisibleSelection in ReplaceSelectionCommand This CL reduces the usage of VisibleSelection in ReplaceSelectionCommand. When querying EndingVisibleSelection or setting the ending selection using SelectionInDOMTree , unnecessary canonicalization is done, which is not needed by ReplaceSelectionCommand. So we can use SelectionForUndoStep to avoid unnecessary calculations. Bug: 657256 Change-Id: Ic18a09ebe6ebc675c9c7b215c035df611cd6f70a Reviewed-on: https://chromium-review.googlesource.com/626216 Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#499455} [modify] https://crrev.com/f0e199c8acaf3becd1f806f3d6f5b1927fab0740/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
,
Sep 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1ef12fb1172dc5120e11b0b5c3c121309ebf7f03 commit 1ef12fb1172dc5120e11b0b5c3c121309ebf7f03 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Mon Sep 11 05:42:02 2017 Make CompositeEditCommand::EndingVisibleSelection as protected EndingVisibleSelection does canonicalization, it's usage outside editor command should not be done. This CL makes EndingVisibleSelection as protected method of CompositeEditCommands. Bug: 657256 Change-Id: I5c5bb9dcf3c35458c0990924a968b57cebf1d897 Reviewed-on: https://chromium-review.googlesource.com/657739 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#500843} [modify] https://crrev.com/1ef12fb1172dc5120e11b0b5c3c121309ebf7f03/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h
,
Sep 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ca06fdc91f50fa2f6c07c51608a5b8039c3c755a commit ca06fdc91f50fa2f6c07c51608a5b8039c3c755a Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Mon Sep 11 06:14:02 2017 Avoid unnecessary canonicalization in InsertListCommand Each EndingVisibleSelection() call creates a new VisibleSelection which unnecessay does canonicalization. We can store the visible selection and use till the selection is not updated. Bug: 657256 Change-Id: I0fa62be89423233e0272d1baef29bf60806bd308 Reviewed-on: https://chromium-review.googlesource.com/657738 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#500848} [modify] https://crrev.com/ca06fdc91f50fa2f6c07c51608a5b8039c3c755a/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp
,
Sep 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4e50c7934b3fd648e7378083761d6fc33b7a3df commit c4e50c7934b3fd648e7378083761d6fc33b7a3df Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Mon Sep 11 12:37:43 2017 Use SelectionForUndoStep in DeleteSelectionCommand DeleteSelectionCommand needs canonicalization. Since we are trying to remove SetEndingSelection(SelectionInDOMTree&) from editor commands, this CL does the in-place changes to calculate the canonicalised values, and then use SelectionForUndoStep for setting command ending and starting selection. Bug: 657256 Change-Id: I5bb2901073c6c48530e097cc78ac323dd8c92ec0 Reviewed-on: https://chromium-review.googlesource.com/641171 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Cr-Commit-Position: refs/heads/master@{#500893} [modify] https://crrev.com/c4e50c7934b3fd648e7378083761d6fc33b7a3df/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp
,
Sep 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f3126a83771cc5846fa5b8ebda9b9028aafb391b commit f3126a83771cc5846fa5b8ebda9b9028aafb391b Author: Xiaocheng Hu <xiaochengh@chromium.org> Date: Mon Sep 11 22:30:15 2017 Fix InputMethodController::IsAvailable and GetDocument The current implementation of InputMethodController::IsAvailable only checks the existence of the frame's document, which can be wrong when the document is detached and inactive. This patch changes it to check LifecycleContext() instead to correct the issue. GetDocument() is also changed accordingly to make the implementations more consistent. This patch actually catches a bug in ComputeWebTextInputNextPreviousFlags that if can be called with inactive document, which is also fixed. Bug: 657256 Change-Id: I56c644ba1e5509fe9e3e89c3c72b28fdceb134b1 Reviewed-on: https://chromium-review.googlesource.com/661162 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Ryan Landay <rlanday@chromium.org> Cr-Commit-Position: refs/heads/master@{#501065} [modify] https://crrev.com/f3126a83771cc5846fa5b8ebda9b9028aafb391b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
,
Sep 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f19336df16023b37605dab5d97055117a087d9f7 commit f19336df16023b37605dab5d97055117a087d9f7 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Fri Sep 15 08:21:36 2017 Handle TypingCommand::InsertText missing cases TypingCommand::InsertText stores the current visible selection of frame, which could get invalidate in certain scenarios like a) event_handler invalidating the frame. b) typing command updating the frame selection. c) even in the case of a subframe the event can remove the frame, which can cause crash. These scenarios are handled in this CL. Bug: 657256 Change-Id: Ib23a93bd72b806d603fb07981f7dd7957fd8817c Reviewed-on: https://chromium-review.googlesource.com/643041 Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#502204} [add] https://crrev.com/f19336df16023b37605dab5d97055117a087d9f7/third_party/WebKit/LayoutTests/editing/inserting/insert-text-nodes-disconnect-on-textinput-event.html [add] https://crrev.com/f19336df16023b37605dab5d97055117a087d9f7/third_party/WebKit/LayoutTests/editing/inserting/insert-text-nodes-disconnect-on-webkitBeforeTextInserted-event.html [add] https://crrev.com/f19336df16023b37605dab5d97055117a087d9f7/third_party/WebKit/LayoutTests/editing/inserting/insert-text-remove-iframe-on-textInput-event.html [add] https://crrev.com/f19336df16023b37605dab5d97055117a087d9f7/third_party/WebKit/LayoutTests/editing/inserting/insert-text-remove-iframe-on-webkitBeforeTextInserted-event.html [modify] https://crrev.com/f19336df16023b37605dab5d97055117a087d9f7/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp
,
Oct 4 2017
,
Oct 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e811ce55fa4652fe3f10489363f1d6192455d692 commit e811ce55fa4652fe3f10489363f1d6192455d692 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Fri Oct 06 05:56:02 2017 Avoid unnecessary canonicalization in edit commands Every call to EndingVisbleSelection does canonicalization. Few edit commands were doing this unnecesarily which has been avoided in this CL. So instead of quering each time with by calling EndingVisibleSelection(), VisibleSelection is stored for the scope and used. Bug: 657256 Change-Id: Iecd2a8feda45fc2962c916369aa7111670ee941a Reviewed-on: https://chromium-review.googlesource.com/702096 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Cr-Commit-Position: refs/heads/master@{#506990} [modify] https://crrev.com/e811ce55fa4652fe3f10489363f1d6192455d692/third_party/WebKit/Source/core/editing/commands/BreakBlockquoteCommand.cpp [modify] https://crrev.com/e811ce55fa4652fe3f10489363f1d6192455d692/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp [modify] https://crrev.com/e811ce55fa4652fe3f10489363f1d6192455d692/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp [modify] https://crrev.com/e811ce55fa4652fe3f10489363f1d6192455d692/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp
,
Oct 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a04a639e647576e9d44eb06c143779456246a3a commit 1a04a639e647576e9d44eb06c143779456246a3a Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Mon Oct 16 15:26:19 2017 Reduce doing canonicalization in TypingCommand TypingCommand does not need to calculate canonicalise values at few places. This CL instead uses SelectionForUndoStep to avoid these calls. This CL also makes TypingCommand not to use SetEndingVisibleSelection, and instead use SelectionForUndoStep for setting ending selection. Bug: 657256 Change-Id: I832945f3a1cd12d3bd06e5700832a3433517e472 Reviewed-on: https://chromium-review.googlesource.com/638110 Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#509049} [modify] https://crrev.com/1a04a639e647576e9d44eb06c143779456246a3a/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp
,
Oct 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/02ed81ce8f9bf4d16e7c9445353c0c457d5ef712 commit 02ed81ce8f9bf4d16e7c9445353c0c457d5ef712 Author: tanvir.rizvi <tanvir.rizvi@samsung.com> Date: Mon Oct 23 10:24:43 2017 Reduce canonicalization in CompositeEditCommand Currently SetStartingSelection,SetEndingSelection are overloaded for SelectionInDOMTree. This CL makes SetStartingSelection and SetEndingSelection to work only with SelectionForUndoStep. Importantly SetEndingSelection(SelectionInDOMTree) does canonicalization, so there were many places which did not needed canonicalization and clean layout, but still was done. After this CL the callers have to ensure if they need to store the EndingSelection as canonicalized value. Subsequently this CL also removes SetEndingVisibleSelection which was a wrapper for SetEndingSelection. Bug: 657256 Change-Id: I34899d5d5bf4daacf122bd85a06865f0f3ca6dd1 Reviewed-on: https://chromium-review.googlesource.com/722839 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com> Cr-Commit-Position: refs/heads/master@{#510752} [modify] https://crrev.com/02ed81ce8f9bf4d16e7c9445353c0c457d5ef712/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/02ed81ce8f9bf4d16e7c9445353c0c457d5ef712/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h
,
Oct 23
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
,
Oct 23
Editing commands no longer store VisibleSelection as start/end selections since the introduction of SelectionForUndoStep. Hence marking as fixed. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Mar 7 2017