Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(184)

Issue 2960473002: [MarkersIntersectingRange #2.1] Add DocumentMarkerController::FirstMarkerIntersectingOffsetRange() (Closed)

Created:
3 years, 6 months ago by rlanday
Modified:
3 years, 5 months ago
Reviewers:
yosin_UTC9, Xiaocheng
CC:
blink-reviews, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add DocumentMarkerController::FirstMarkerIntersectingOffsetRange() This is a method that takes a text node, a pair of start/end offsets, and a list of MarkerTypes, and tries to find a DocumentMarker in the node of one of those types that intersects the range [start_offset, end_offset]. This is similar to DocumentMarkerController::MarkersIntersectingRange(), which I'm introducing in https://codereview.chromium.org/2948133004, except that this method takes a Text node and offsets instead of taking an EphemeralRange/ EphemeralRangeInFlatTree, and only returns at most one DocumentMarker, instead of returning all of them that match. BUG=707867 Review-Url: https://codereview.chromium.org/2960473002 Cr-Commit-Position: refs/heads/master@{#488549} Committed: https://chromium.googlesource.com/chromium/src/+/a959e15bb4141aaf7041b2bbfd42f2d6d72cfc66

Patch Set 1 #

Patch Set 2 : Update comment, Empty => Collapsed #

Patch Set 3 : Rebase #

Total comments: 1

Patch Set 4 : Rebase #

Patch Set 5 : Rename method #

Patch Set 6 : Remove dependency #

Patch Set 7 : Rebase #

Patch Set 8 : Rename MarkerIntersectingCollapsedRange test to FirstMarkerIntersectingOffsetRange_collapsed #

Total comments: 2

Patch Set 9 : Use new DocumentMarkerList::FirstMarkerIntersectingRange() method #

Total comments: 1

Patch Set 10 : Try to clarify comment #

Patch Set 11 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+78 lines, -0 lines) Patch
M third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h View 1 2 3 4 5 6 7 8 9 1 chunk +11 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp View 1 2 3 4 5 6 7 8 1 chunk +26 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp View 1 2 3 4 5 6 7 1 chunk +41 lines, -0 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 57 (42 generated)
rlanday
3 years, 6 months ago (2017-06-23 21:36:35 UTC) #4
Xiaocheng
On 2017/06/23 at 21:36:35, rlanday wrote: > Sorry I'm lost... Too many patches... Could you ...
3 years, 6 months ago (2017-06-23 22:13:23 UTC) #5
rlanday
On 2017/06/23 at 22:13:23, xiaochengh wrote: > On 2017/06/23 at 21:36:35, rlanday wrote: > > ...
3 years, 6 months ago (2017-06-23 22:25:15 UTC) #6
rlanday
On 2017/06/23 at 22:25:15, rlanday wrote: > - Make ContextMenuClient::ReplaceMisspelledRange() use GetSpellCheckMarkerTouchingSelection() sorry, I meant ...
3 years, 6 months ago (2017-06-23 22:27:07 UTC) #7
Xiaocheng
No problem with code change. https://codereview.chromium.org/2960473002/diff/40001/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h File third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h (right): https://codereview.chromium.org/2960473002/diff/40001/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h#newcode101 third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h:101: DocumentMarker* MarkerIntersectingRange(const Text&, I ...
3 years, 5 months ago (2017-06-26 22:44:18 UTC) #12
rlanday
On 2017/06/26 at 22:44:18, xiaochengh wrote: > No problem with code change. > > https://codereview.chromium.org/2960473002/diff/40001/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h ...
3 years, 5 months ago (2017-06-28 01:59:44 UTC) #15
rlanday
On 2017/06/28 at 01:59:44, rlanday wrote: > On 2017/06/26 at 22:44:18, xiaochengh wrote: > > ...
3 years, 5 months ago (2017-07-05 18:30:32 UTC) #17
yosin_UTC9
https://codereview.chromium.org/2960473002/diff/140001/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp File third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp (right): https://codereview.chromium.org/2960473002/diff/140001/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp#newcode419 third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp:419: list->MarkersIntersectingRange(start_offset, end_offset); We should have DocumentMarjerList::FirstMarkerIntersectingOffsetRange() instead of taking ...
3 years, 5 months ago (2017-07-20 01:07:01 UTC) #34
rlanday
On 2017/07/20 at 01:07:01, yosin wrote: > https://codereview.chromium.org/2960473002/diff/140001/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp > File third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp (right): > > https://codereview.chromium.org/2960473002/diff/140001/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp#newcode419 ...
3 years, 5 months ago (2017-07-20 18:38:47 UTC) #37
Xiaocheng
The function impl is fine, but there's an issue of the specified behavior of the ...
3 years, 5 months ago (2017-07-20 21:13:49 UTC) #40
rlanday
On 2017/07/20 at 21:13:49, xiaochengh wrote: > The function impl is fine, but there's an ...
3 years, 5 months ago (2017-07-20 21:22:38 UTC) #43
Xiaocheng
All right, lgtm then
3 years, 5 months ago (2017-07-20 21:35:37 UTC) #44
yosin_UTC9
lgtm
3 years, 5 months ago (2017-07-21 00:58:49 UTC) #49
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2960473002/200001
3 years, 5 months ago (2017-07-21 01:54:21 UTC) #54
commit-bot: I haz the power
3 years, 5 months ago (2017-07-21 02:00:01 UTC) #57
Message was sent while issue was closed.
Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/a959e15bb4141aaf7041b2bbfd42...

Powered by Google App Engine
This is Rietveld 408576698