New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 676445 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 673491



Sign in to add a comment

DocumentMarkerController::addMarker(Node* node, const DocumentMarker& newMarker) is buggy

Project Member Reported by rlanday@chromium.org, Dec 21 2016

Issue description

I 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.
 
Project Member

Comment 2 by bugdroid1@chromium.org, 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

Status: Fixed (was: Assigned)

Sign in to add a comment