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

Issue 1190303002: Introduce Position::nodeAsRangeLastNode (Closed)

Created:
5 years, 6 months ago by hajimehoshi
Modified:
5 years, 6 months ago
Reviewers:
tkent, yosin_UTC9
CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, sof, eae+blinkwatch, rwlbuis
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Introduce Position::nodeAsRangeLastNode This CL introduces Position::nodeAsRangeLastNode which returns a previous node of nodeAsRangePastLastNode. This function will be used at VisibleSelection for the composed tree (crrev.com/1194833002). This CL also adds tests for Position. This CL is a split of crrev.com/1194833002. This patch is result of collaboration work with yosin@chromium.org for selection of web components. We've already prepared the new node-traversal usages at crrev.com/1188063002. BUG=275851 TEST=webkit_unit_tests --gtest_filter=PositionTest.* Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197554

Patch Set 1 #

Total comments: 4

Patch Set 2 : reviews #

Unified diffs Side-by-side diffs Delta from patch set Stats (+139 lines, -0 lines) Patch
M Source/core/core.gypi View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/dom/Position.h View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/dom/Position.cpp View 1 chunk +10 lines, -0 lines 0 comments Download
A Source/core/dom/PositionTest.cpp View 1 1 chunk +125 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (5 generated)
hajimehoshi
PTAL
5 years, 6 months ago (2015-06-19 10:08:55 UTC) #2
yosin_UTC9
Code changes looks OK. https://codereview.chromium.org/1190303002/diff/1/Source/core/dom/PositionTest.cpp File Source/core/dom/PositionTest.cpp (right): https://codereview.chromium.org/1190303002/diff/1/Source/core/dom/PositionTest.cpp#newcode12 Source/core/dom/PositionTest.cpp:12: #include "wtf/RefPtr.h" Please include "core/testing/CoreTestHelpers.h", ...
5 years, 6 months ago (2015-06-19 11:59:01 UTC) #3
tkent
lgtm https://codereview.chromium.org/1190303002/diff/1/Source/core/dom/PositionTest.cpp File Source/core/dom/PositionTest.cpp (right): https://codereview.chromium.org/1190303002/diff/1/Source/core/dom/PositionTest.cpp#newcode125 Source/core/dom/PositionTest.cpp:125: nit: this blank line is unnecessary.
5 years, 6 months ago (2015-06-22 01:03:25 UTC) #4
hajimehoshi
Thank you! https://codereview.chromium.org/1190303002/diff/1/Source/core/dom/PositionTest.cpp File Source/core/dom/PositionTest.cpp (right): https://codereview.chromium.org/1190303002/diff/1/Source/core/dom/PositionTest.cpp#newcode12 Source/core/dom/PositionTest.cpp:12: #include "wtf/RefPtr.h" On 2015/06/19 11:59:00, Yosi_UTC9 wrote: ...
5 years, 6 months ago (2015-06-22 04:09:08 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1190303002/20001
5 years, 6 months ago (2015-06-22 04:09:23 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/59992)
5 years, 6 months ago (2015-06-22 05:25:42 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1190303002/20001
5 years, 6 months ago (2015-06-22 05:26:24 UTC) #12
commit-bot: I haz the power
5 years, 6 months ago (2015-06-22 06:26:27 UTC) #13
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197554

Powered by Google App Engine
This is Rietveld 408576698