DocumentMarkerController::addMarker(Node* node, const DocumentMarker& newMarker) is buggy |
||
Issue descriptionI discovered this when trying to add support for persistent CompositionUnderline spans to support the Android Voice IME. DocumentMarkerController::m_markers is a hash map from Node* to a vector (indexed by DocumentMarker::MarkerIndexType) of vectors of DocumentMarkers. I added a new MarkerType and MarkerTypeIndex for a persisting CompositionMarker. I spent several hours trying to determine why my markers were getting cleared despite having commented out all the methods I could find for removing them and found this bug. There's a check in addMarker() to see if we've created a MarkerList for a given MarkerTypeIndex. If it fails (e.g., we've been inserting a bunch of PersistingCompositionMarkers and we want to insert a CompositionMarker), Vector::insert() is called to insert a new list: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp?cl=GROK&gsn=MarkerTypeIndexesCount&rcl=1482337321&pv=1&l=247 Unfortunately this is the wrong operation; insert() shifts all the successor elements so they're now in the wrong positions. So now when we look up the list of PersistingCompositionMarkers, we can't find it since it's in the wrong place, and the markers basically just disappear.
,
Jan 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/23142d0b7ace418a54fb65bbb9be72ac4e79be86 commit 23142d0b7ace418a54fb65bbb9be72ac4e79be86 Author: rlanday <rlanday@chromium.org> Date: Thu Jan 05 20:01:21 2017 Fix bug in DocumentMarkerController::addMarker(Node* node, const DocumentMarker& newMarker) Basically it's doing a vector insert when it should be doing an element assignment. See crbug.com/676445 for details BUG= 676445 Review-Url: https://codereview.chromium.org/2594123003 Cr-Commit-Position: refs/heads/master@{#441740} [modify] https://crrev.com/23142d0b7ace418a54fb65bbb9be72ac4e79be86/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
,
Jan 5 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by rlanday@chromium.org
, Dec 21 2016