Yoshifumi Inoue would like Xiaocheng Hu and Yoichi Osato to review this change.
Make TextIteratorTextState::EmitText() to take const Text&
This patch changes |TextIteratorTextState::EmitText()| to take |const Text&|
since it doesn't take |nullptr| and all call sites passed non-null |Text| node
for improving code health.
Change-Id: I6ef1fb5d5dc85f0e4edc16be3d27913ac73648d4
---
M third_party/blink/renderer/core/editing/iterators/simplified_backwards_text_iterator.cc
M third_party/blink/renderer/core/editing/iterators/text_iterator_text_node_handler.cc
M third_party/blink/renderer/core/editing/iterators/text_iterator_text_state.cc
M third_party/blink/renderer/core/editing/iterators/text_iterator_text_state.h
4 files changed, 12 insertions(+), 9 deletions(-)
To view, visit change 1025452. To unsubscribe, or for help writing mail filters, visit settings.
PTAL
LGTM to unblock further actions.
Patch set 3:Code-Review +1
1 comment:
File third_party/blink/renderer/core/editing/iterators/text_iterator_text_state.cc:
Patch Set #3, Line 159: DCHECK_LE(position_end_offset, text_node.length());
Unfortunately, this is broken by CSS property text-transform...
Let's comment this DCHECK and add a TODO about text-transform.
To view, visit change 1025452. To unsubscribe, or for help writing mail filters, visit settings.
Thanks for reviewing!
Committing...
Patch set 4:Commit-Queue +2
1 comment:
File third_party/blink/renderer/core/editing/iterators/text_iterator_text_state.cc:
Patch Set #3, Line 159: // TODO(editing-dev): text-transform:uppercase can make text longer, e.g.
Unfortunately, this is broken by CSS property text-transform... […]
Done
To view, visit change 1025452. To unsubscribe, or for help writing mail filters, visit settings.
CQ is trying the patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"2018-04-25T08:49:53" https://chromium-review.googlesource.com/c/1025452/4
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/1025452/4
Bot data: {"action": "start", "triggered_at": "2018-04-24T23:51:05.0Z", "cq_cfg_revision": "ec7cec62c0feefff61026ab368fbea1f7be53e60", "revision": "1ba3249988472c4cd27a987988eaf33cff6fd58e"}
Try jobs failed on following builders:
linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/536112)
Patch set 4:Code-Review +1
Commit Bot merged this change.
Make TextIteratorTextState::EmitText() to take const Text&
This patch changes |TextIteratorTextState::EmitText()| to take |const Text&|
since it doesn't take |nullptr| and all call sites passed non-null |Text| node
for improving code health.
Change-Id: I6ef1fb5d5dc85f0e4edc16be3d27913ac73648d4
Reviewed-on: https://chromium-review.googlesource.com/1025452
Reviewed-by: Yoichi Osato <yoi...@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaoc...@chromium.org>
Commit-Queue: Yoshifumi Inoue <yo...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553425}
---
M third_party/blink/renderer/core/editing/iterators/simplified_backwards_text_iterator.cc
M third_party/blink/renderer/core/editing/iterators/text_iterator_text_node_handler.cc
M third_party/blink/renderer/core/editing/iterators/text_iterator_text_state.cc
M third_party/blink/renderer/core/editing/iterators/text_iterator_text_state.h
4 files changed, 14 insertions(+), 9 deletions(-)