VisiblePositions should be used only when valid |
||||||||
Issue descriptionVisiblePositions are short-living objects that are created with clean layout and invalidated as soon as there is a DOM or style change. Unfortunately, there are many clients that store a VisiblePosition, do some mutation and then inspect the VisiblePosition's property. We should stop doing that.
,
Sep 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/58d3e91f04c7eb7847dade2b3f427da26dfd7379 commit 58d3e91f04c7eb7847dade2b3f427da26dfd7379 Author: xiaochengh <xiaochengh@chromium.org> Date: Tue Sep 27 01:04:32 2016 Mark paragraph-related functions deprecated in VisibleUnits This patch marks the following functions in VisibleUnits.cpp deprecated because they accept invalid VisiblePositions as input: - startOfParagraph - endOfParagraph - isStartOfParagraph - isEndOfParagraph - inSameParagraph - startOfNextParagraph This patch also creates the non-deprecated versions of these functions that requires valid VisiblePositions as input. Calls of these functions in editing/commands/ are switched to the deprecated functions. Calls in other places have ensured valid VisiblePositions as input, and hence, are calling the non-deprecated functions after this patch. BUG=648949 Review-Url: https://codereview.chromium.org/2366693005 Cr-Commit-Position: refs/heads/master@{#421063} [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/VisibleUnits.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/VisibleUnits.h [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/FormatBlockCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/IndentOutdentCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/InsertLineBreakCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp [modify] https://crrev.com/58d3e91f04c7eb7847dade2b3f427da26dfd7379/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp
,
Sep 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1985713ab7e640a5d5de02a579e10aed85eae34a commit 1985713ab7e640a5d5de02a579e10aed85eae34a Author: xiaochengh <xiaochengh@chromium.org> Date: Tue Sep 27 01:08:02 2016 Add VisiblePosition validity checks to VisibleUnits.cpp BUG=648949 Review-Url: https://codereview.chromium.org/2363363002 Cr-Commit-Position: refs/heads/master@{#421064} [modify] https://crrev.com/1985713ab7e640a5d5de02a579e10aed85eae34a/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
,
Sep 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/172391bd511c3871ff8c7bcd73fa9cb5c1d12043 commit 172391bd511c3871ff8c7bcd73fa9cb5c1d12043 Author: xiaochengh <xiaochengh@chromium.org> Date: Tue Sep 27 07:37:52 2016 Ensure valid VisiblePosition input for inSameBlock This patch postpones the creation of |visiblePos| in |InsertParagraphSeparatorCommand::doApply| so that |visiblePos| does not get invalidated by the creation of |blockToInsert|. Note: ideally, the creation and modification of an out-of-document element (in this case, |blockToInsert|) shouldn't invalidate VisiblePositions, but this is still hard to fix in the current implementation. See crbug.com/650164 . BUG=648949 Review-Url: https://codereview.chromium.org/2372903003 Cr-Commit-Position: refs/heads/master@{#421138} [modify] https://crrev.com/172391bd511c3871ff8c7bcd73fa9cb5c1d12043/third_party/WebKit/Source/core/editing/VisibleUnits.cpp [modify] https://crrev.com/172391bd511c3871ff8c7bcd73fa9cb5c1d12043/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp
,
Sep 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b0d88beaef6c050b97d2aacd3310be4ef6fd4c0 commit 7b0d88beaef6c050b97d2aacd3310be4ef6fd4c0 Author: xiaochengh <xiaochengh@chromium.org> Date: Tue Sep 27 10:45:03 2016 Ensure valid VisiblePosition input for character{Before,After} This patch also drive-by eliminates a |createVisiblePositionDeprecated| in |CompositeEditCommand::prepareWhitespaceAtPositionForSplit|. BUG=648949 Review-Url: https://codereview.chromium.org/2368903002 Cr-Commit-Position: refs/heads/master@{#421164} [modify] https://crrev.com/7b0d88beaef6c050b97d2aacd3310be4ef6fd4c0/third_party/WebKit/Source/core/editing/VisibleUnits.cpp [modify] https://crrev.com/7b0d88beaef6c050b97d2aacd3310be4ef6fd4c0/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/7b0d88beaef6c050b97d2aacd3310be4ef6fd4c0/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
,
Sep 28 2016
,
Sep 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4ebc66c03af35f9294189171d84ebf7c4ba348a3 commit 4ebc66c03af35f9294189171d84ebf7c4ba348a3 Author: xiaochengh <xiaochengh@chromium.org> Date: Wed Sep 28 01:20:09 2016 Mark calls of {previous,next}PositionOf with invalid VisiblePosition deprecated This patch marks functions {previous,next}PositionOf in VisibleUnits.cpp deprecated because they take invalid VisiblePositions as input. This patch also creates the non-deprecated versions of these functions that requires valid VisiblePositions as input. Callers passing invalid VisiblePositions have been switched to using the deprecated functions. Calls in other places have ensured valid VisiblePositions as input, and hence, are calling the non-deprecated functions after this patch. BUG=648949 Review-Url: https://codereview.chromium.org/2371793003 Cr-Commit-Position: refs/heads/master@{#421414} [modify] https://crrev.com/4ebc66c03af35f9294189171d84ebf7c4ba348a3/third_party/WebKit/Source/core/editing/VisibleUnits.cpp [modify] https://crrev.com/4ebc66c03af35f9294189171d84ebf7c4ba348a3/third_party/WebKit/Source/core/editing/VisibleUnits.h [modify] https://crrev.com/4ebc66c03af35f9294189171d84ebf7c4ba348a3/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/4ebc66c03af35f9294189171d84ebf7c4ba348a3/third_party/WebKit/Source/core/editing/commands/IndentOutdentCommand.cpp
,
Sep 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c7fdeab7c9ee00f1ea57b71ee5341109280b778e commit c7fdeab7c9ee00f1ea57b71ee5341109280b778e Author: xiaochengh <xiaochengh@chromium.org> Date: Wed Sep 28 01:29:42 2016 Ensure valid VisiblePosition input for absoluteCaretBoundsOf BUG=648949 Review-Url: https://codereview.chromium.org/2373483003 Cr-Commit-Position: refs/heads/master@{#421418} [modify] https://crrev.com/c7fdeab7c9ee00f1ea57b71ee5341109280b778e/third_party/WebKit/Source/core/editing/VisibleUnits.cpp [modify] https://crrev.com/c7fdeab7c9ee00f1ea57b71ee5341109280b778e/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp
,
Oct 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0895689a9cf3613ea07cd1a05ce4c6d32d6d411c commit 0895689a9cf3613ea07cd1a05ce4c6d32d6d411c Author: xiaochengh <xiaochengh@chromium.org> Date: Mon Oct 03 09:32:07 2016 Make DragCaretController store PositionWithAffinity instead of VisiblePosition DragCaretController is an on-stack object that stores a VisiblePosition and may inspect properties of the VisiblePosition even after it is invalidated. This patch fixes it by making DragCaretController store a PositionWithAffinity instead. This patch is a preparation of pruning createVisibleSelectionDeprecated from DragController: https://codereview.chromium.org/2386083002 BUG=648949, 651373 Review-Url: https://codereview.chromium.org/2388703002 Cr-Commit-Position: refs/heads/master@{#422395} [modify] https://crrev.com/0895689a9cf3613ea07cd1a05ce4c6d32d6d411c/third_party/WebKit/Source/core/editing/DragCaretController.cpp [modify] https://crrev.com/0895689a9cf3613ea07cd1a05ce4c6d32d6d411c/third_party/WebKit/Source/core/editing/DragCaretController.h
,
Oct 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cf43a0661cc568c57f073904d76b989b363b23ea commit cf43a0661cc568c57f073904d76b989b363b23ea Author: xiaochengh <xiaochengh@chromium.org> Date: Fri Oct 07 05:36:48 2016 Prune deprecated functions from TypingCommand BUG=648949, 651373 Review-Url: https://codereview.chromium.org/2393353004 Cr-Commit-Position: refs/heads/master@{#423801} [modify] https://crrev.com/cf43a0661cc568c57f073904d76b989b363b23ea/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp
,
Oct 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1947c7fd01731ba28e72137e99d9cf78c7ab0b6 commit d1947c7fd01731ba28e72137e99d9cf78c7ab0b6 Author: xiaochengh <xiaochengh@chromium.org> Date: Fri Oct 07 09:19:23 2016 Prune deprecated functions from InsertListCommand BUG= 647219 ,648949, 651373 Review-Url: https://codereview.chromium.org/2400053003 Cr-Commit-Position: refs/heads/master@{#423822} [modify] https://crrev.com/d1947c7fd01731ba28e72137e99d9cf78c7ab0b6/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp
,
Oct 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/498e9e19dffeb563c02167e40da7a4424eaba02e commit 498e9e19dffeb563c02167e40da7a4424eaba02e Author: xiaochengh <xiaochengh@chromium.org> Date: Tue Oct 11 08:32:06 2016 Prune deprecated functions from InsertTextCommand BUG= 647219 ,648949, 651373 Review-Url: https://codereview.chromium.org/2404183002 Cr-Commit-Position: refs/heads/master@{#424392} [modify] https://crrev.com/498e9e19dffeb563c02167e40da7a4424eaba02e/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp
,
Oct 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7850cba844bb06053c9d6d553700240d15978af4 commit 7850cba844bb06053c9d6d553700240d15978af4 Author: xiaochengh <xiaochengh@chromium.org> Date: Wed Oct 12 05:01:16 2016 Recalculate |visiblePos| for InsertParagraphSeparatorCommand::doApply() after mutations This patch does the following change to the above mentioned function that, |visiblePos| is always recalculated as |createVisiblePosition(insertionPosition)| before being used, so that it is always valid at those places. BUG=648949 Review-Url: https://codereview.chromium.org/2412443002 Cr-Commit-Position: refs/heads/master@{#424673} [modify] https://crrev.com/7850cba844bb06053c9d6d553700240d15978af4/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp
,
Oct 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5f0ab5ec41d132eb4ff36460e4f05c110dda17f6 commit 5f0ab5ec41d132eb4ff36460e4f05c110dda17f6 Author: xiaochengh <xiaochengh@chromium.org> Date: Wed Oct 12 06:25:14 2016 Ensure valid input for CompositeEditCommand::moveParagraph[s] This patch ensured valid VisiblePositions as input arguments to CompositeEditCommand's moveParagraph() and moveParagraphs(), and hence, prunes the remaining deprecated functions therein. BUG=648949 Review-Url: https://codereview.chromium.org/2405223002 Cr-Commit-Position: refs/heads/master@{#424682} [modify] https://crrev.com/5f0ab5ec41d132eb4ff36460e4f05c110dda17f6/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/5f0ab5ec41d132eb4ff36460e4f05c110dda17f6/third_party/WebKit/Source/core/editing/commands/IndentOutdentCommand.cpp [modify] https://crrev.com/5f0ab5ec41d132eb4ff36460e4f05c110dda17f6/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp [modify] https://crrev.com/5f0ab5ec41d132eb4ff36460e4f05c110dda17f6/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp
,
Oct 12 2016
Currently, there is a lot of usages of invalidated VisiblePositions in editing/commands. To get rid of them: 1. EditCommand should stop storing VisibleSelections as its starting & ending selection. Instead, it should store a somehow lightweight class like SelectionInDOMTree 2. Currently EditCommands (and also a bunch of functions in VisibleUnits) are abusing VisiblePositions. Actually, they only require canonical positions (probably with affinity) as input. We should make them pass canonical positions instead of VPs. 3. Eliminate all VPs stored on heap as they will definitely become invalidated over time. 4. All stack allocated VPs should be stored as const references. New VPs should be created after DOM/style changes.
,
Oct 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c7a2dfb0e4118e118fdbd2bd566aa3fa77a7d665 commit c7a2dfb0e4118e118fdbd2bd566aa3fa77a7d665 Author: xiaochengh <xiaochengh@chromium.org> Date: Wed Oct 12 14:03:10 2016 Prune deprecated functions from VisibleSelection and VisibleUnits This patch prunes all deprecated functions from VisibleSelection and VisibleUnits because they have no callers any more. The following functions are removed: - VisibleSelection::visible{Start,End}Deprecated - createVisibleSelectionDeprecated (all overloads) - paragraph-related deprecated functions (in VisibleUnits) - {previous,next}PositionOf (in VisibleUnits) BUG= 647219 ,648949, 651373 Review-Url: https://codereview.chromium.org/2411303002 Cr-Commit-Position: refs/heads/master@{#424732} [modify] https://crrev.com/c7a2dfb0e4118e118fdbd2bd566aa3fa77a7d665/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/c7a2dfb0e4118e118fdbd2bd566aa3fa77a7d665/third_party/WebKit/Source/core/editing/VisibleSelection.h [modify] https://crrev.com/c7a2dfb0e4118e118fdbd2bd566aa3fa77a7d665/third_party/WebKit/Source/core/editing/VisibleUnits.cpp [modify] https://crrev.com/c7a2dfb0e4118e118fdbd2bd566aa3fa77a7d665/third_party/WebKit/Source/core/editing/VisibleUnits.h
,
Oct 19 2016
,
Oct 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d7c76ec65a164e8b0d1bcede4f6763026ce29042 commit d7c76ec65a164e8b0d1bcede4f6763026ce29042 Author: yosin <yosin@chromium.org> Date: Wed Oct 26 04:04:48 2016 Introduce EditCommand::setEndingSelection() taking SelectionInDOMTree This patch introduces |SelectionInDOMTree| version of |setEndingSelection()| in |EditCommand| class as a preparation of getting rid of overloads and update layout calls for improving code health. Following patch will introduce SelectionInUndoStep which is intended for restore selection after DOM tree mutation at undo/redo time. BUG=648949, 646323 TEST=n/a; no behavior changes Review-Url: https://codereview.chromium.org/2450603003 Cr-Commit-Position: refs/heads/master@{#427585} [modify] https://crrev.com/d7c76ec65a164e8b0d1bcede4f6763026ce29042/third_party/WebKit/Source/core/editing/commands/EditCommand.cpp [modify] https://crrev.com/d7c76ec65a164e8b0d1bcede4f6763026ce29042/third_party/WebKit/Source/core/editing/commands/EditCommand.h
,
Apr 28 2017
,
Jul 29 2017
,
Oct 4 2017
,
Oct 4
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 5
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by xiaoche...@chromium.org
, Sep 26 2016