New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment
link

Issue 868227: Improve TextFinder

Reported by rakina@chromium.org, Jul 27 2018 Project Member

Issue description

Currently TextFinder's structure is a bit confusing. We want to refactor/rewrite TextFinder.

See https://docs.google.com/document/d/12Hqv8KCwqq1_aWh0Djx-rvIAgbwdWVS80HkSnYS2RCs/edit?usp=sharing
 

Comment 1 by bugdroid1@chromium.org, Aug 17

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d4ce97f15ace5e4431e8702fdce563fe67299b2f

commit d4ce97f15ace5e4431e8702fdce563fe67299b2f
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Fri Aug 17 12:51:42 2018

Make TextFinder Scoping use Idle Tasks

Currently scoping (searching for all find matches) is done in 100ms
chunks and uses timers. We want to make it use idle tasks instead to
ensure this doesn't block important tasks.

Bug: 868227
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Iaa9f40580fe5c69af398ae43a339d06305767503
Reviewed-on: https://chromium-review.googlesource.com/1152710
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584034}
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/chrome/browser/chrome_find_request_manager_browsertest.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/chrome/browser/ui/find_bar/find_tab_helper.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/chrome/browser/ui/find_bar/find_tab_helper.h
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/chrome/test/base/ui_test_utils.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/content/browser/download/mhtml_generation_browsertest.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/content/browser/find_request_manager.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/content/browser/find_request_manager_browsertest.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/content/common/frame_messages.h
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/third_party/blink/public/mojom/frame/find_in_page.mojom
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/third_party/blink/public/web/web_find_options.h
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/third_party/blink/renderer/core/editing/finder/text_finder.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/third_party/blink/renderer/core/editing/finder/text_finder.h
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/third_party/blink/renderer/core/editing/finder/text_finder_test.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/third_party/blink/renderer/core/frame/find_in_page.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/third_party/blink/renderer/core/frame/find_in_page_test.cc
[modify] https://crrev.com/d4ce97f15ace5e4431e8702fdce563fe67299b2f/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc

Comment 2 by bugdroid1@chromium.org, Aug 29

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9060444eb79076dfa07cb6bdee84084c4369ed7

commit d9060444eb79076dfa07cb6bdee84084c4369ed7
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Wed Aug 29 19:25:20 2018

Add UMA for time over deadline in Find-in-Page Scoping

Bug: 868227
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I4dc042dbcf7e38b71a64d1caa2982110e2796fef
Reviewed-on: https://chromium-review.googlesource.com/1193602
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587239}
[modify] https://crrev.com/d9060444eb79076dfa07cb6bdee84084c4369ed7/third_party/blink/renderer/core/editing/finder/text_finder.cc
[modify] https://crrev.com/d9060444eb79076dfa07cb6bdee84084c4369ed7/tools/metrics/histograms/histograms.xml

Comment 3 by bugdroid1@chromium.org, Oct 15

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/153bb13190554713cd65d1c48eee5ae69b2a609f

commit 153bb13190554713cd65d1c48eee5ae69b2a609f
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Mon Oct 15 10:06:04 2018

Move Find-in-page scoping to FindTaskController

This CL moves find-in-page scoping methods and idle task to a new
class, FindTaskController and use the term "finding" instead of
"scoping" there. FindTaskController is responsible for managing
IdleFindTask, and notifying TextFinder of matches etc, so that
TextFinder can manage markers, tickmark, notifying frame etc.

Bug: 868227
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I20701ec0542acb40ace471891a490612e73d700b
Reviewed-on: https://chromium-review.googlesource.com/c/1278074
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599605}
[modify] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/DEPS
[modify] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/BUILD.gn
[add] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/finder/find_task_controller.cc
[add] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/finder/find_task_controller.h
[modify] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/finder/text_finder.cc
[modify] https://crrev.com/153bb13190554713cd65d1c48eee5ae69b2a609f/third_party/blink/renderer/core/editing/finder/text_finder.h

Comment 4 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71e551ac476ec9b247be8f19c6eb696632d90cf4

commit 71e551ac476ec9b247be8f19c6eb696632d90cf4
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Fri Dec 14 16:15:49 2018

Find-in-page per block using FindBuffer

This CL introduces FindBuffer, which Find-in-page will use to find
matches instead of using FindPlainText that uses TextIterator.

Since find-in-page matches can't span multiple blocks, we are using
that fact to know where we can separate the text in the document.
FindBuffer collects text one block at a time, and then finds the match
in that block. We trigger FindBuffer text collection & finding many
times until it goes through everything. FindInPage uses TextSearcherICU
to find text, and uses flat-tree traversal, ComputedStyle, and
LayoutObject + NGOffsetMapping, whereas the previous way was to use
TextIterator.

This CL also changes the way we handle newlines - newlines will be
saved as object replacement character/whitespaces for Find-in-page
purposes. Some tests that rely on newlines are deleted as a result.
Note:
This change is not user-visible because it is not possible to do a
Find-in-page from the UI with newlines.

See doc for details:
https://docs.google.com/document/d/1D5q7ZMrLPfilXnIGI3SOL0mwdiQC7SQEtGWk-9ysbMg/edit?usp=sharing

Bug: 868227
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Id5dd7e6c428f59442d02ec88ea0cec6eb4e64c22
Reviewed-on: https://chromium-review.googlesource.com/c/1282425
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616701}
[modify] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc
[modify] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/chrome/test/data/find_in_page/LongFind.txt
[modify] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/BUILD.gn
[add] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/finder/find_buffer.cc
[add] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/finder/find_buffer.h
[add] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc
[modify] https://crrev.com/71e551ac476ec9b247be8f19c6eb696632d90cf4/third_party/blink/renderer/core/editing/finder/find_task_controller.cc

Comment 5 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/602a22a3a589f4f67bb63669f1ef778530ba5596

commit 602a22a3a589f4f67bb63669f1ef778530ba5596
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Fri Dec 14 17:18:50 2018

Remove unnecessary DCHECK for FIP start position

The DCHECK is actually faulty, because the start position
can be the last position instead in some cases like <hr>

TBR=yosin@chromium.org

Bug: 868227
Change-Id: I9ee87d4cfd710e07742b9c8896583be060816c56
Reviewed-on: https://chromium-review.googlesource.com/c/1378153
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616719}
[modify] https://crrev.com/602a22a3a589f4f67bb63669f1ef778530ba5596/third_party/blink/renderer/core/editing/finder/find_buffer.cc

Comment 6 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c484a22d01e9482be2997dea811367505556a86

commit 9c484a22d01e9482be2997dea811367505556a86
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Fri Dec 14 18:34:33 2018

Revert "Find-in-page per block using FindBuffer"

This reverts commit 71e551ac476ec9b247be8f19c6eb696632d90cf4.

Reason for revert: broke MHTMLGenerationTest.ViewedMHTMLContainsNoStoreContentIfNoCacheControlPolicy on win7 tests dbg: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/73554

[7424:304:1214/084019.035:FATAL:find_buffer.cc(27)] Check failed: start_position == PositionInFlatTree::FirstPositionInNode( *start_position.ComputeContainerNode()).

Original change's description:
> Find-in-page per block using FindBuffer
>
> This CL introduces FindBuffer, which Find-in-page will use to find
> matches instead of using FindPlainText that uses TextIterator.
>
> Since find-in-page matches can't span multiple blocks, we are using
> that fact to know where we can separate the text in the document.
> FindBuffer collects text one block at a time, and then finds the match
> in that block. We trigger FindBuffer text collection & finding many
> times until it goes through everything. FindInPage uses TextSearcherICU
> to find text, and uses flat-tree traversal, ComputedStyle, and
> LayoutObject + NGOffsetMapping, whereas the previous way was to use
> TextIterator.
>
> This CL also changes the way we handle newlines - newlines will be
> saved as object replacement character/whitespaces for Find-in-page
> purposes. Some tests that rely on newlines are deleted as a result.
> Note:
> This change is not user-visible because it is not possible to do a
> Find-in-page from the UI with newlines.
>
> See doc for details:
> https://docs.google.com/document/d/1D5q7ZMrLPfilXnIGI3SOL0mwdiQC7SQEtGWk-9ysbMg/edit?usp=sharing
>
> Bug: 868227
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
> Change-Id: Id5dd7e6c428f59442d02ec88ea0cec6eb4e64c22
> Reviewed-on: https://chromium-review.googlesource.com/c/1282425
> Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#616701}

TBR=ellyjones@chromium.org,yosin@chromium.org,yoichio@chromium.org,kojii@chromium.org,xiaochengh@chromium.org,rakina@chromium.org

Change-Id: Ic06a33d1c630bb4423fc0963bc55b6af5e8ef865
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 868227
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/c/1378407
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616757}
[modify] https://crrev.com/9c484a22d01e9482be2997dea811367505556a86/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc
[modify] https://crrev.com/9c484a22d01e9482be2997dea811367505556a86/chrome/test/data/find_in_page/LongFind.txt
[modify] https://crrev.com/9c484a22d01e9482be2997dea811367505556a86/third_party/blink/renderer/core/editing/BUILD.gn
[delete] https://crrev.com/dacd59c9179d75f89a27be539dde8892984612bc/third_party/blink/renderer/core/editing/finder/find_buffer.cc
[delete] https://crrev.com/dacd59c9179d75f89a27be539dde8892984612bc/third_party/blink/renderer/core/editing/finder/find_buffer.h
[delete] https://crrev.com/dacd59c9179d75f89a27be539dde8892984612bc/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc
[modify] https://crrev.com/9c484a22d01e9482be2997dea811367505556a86/third_party/blink/renderer/core/editing/finder/find_task_controller.cc

Comment 7 by bugdroid1@chromium.org, Dec 15

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0be1119f808d95d053e7368aa3352b7de29ecc87

commit 0be1119f808d95d053e7368aa3352b7de29ecc87
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Sat Dec 15 00:37:08 2018

Reland "Find-in-page per block using FindBuffer"

This reverts commit 9c484a22d01e9482be2997dea811367505556a86.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Revert "Find-in-page per block using FindBuffer"
> 
> This reverts commit 71e551ac476ec9b247be8f19c6eb696632d90cf4.
> 
> Reason for revert: broke MHTMLGenerationTest.ViewedMHTMLContainsNoStoreContentIfNoCacheControlPolicy on win7 tests dbg: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/73554
> 
> [7424:304:1214/084019.035:FATAL:find_buffer.cc(27)] Check failed: start_position == PositionInFlatTree::FirstPositionInNode( *start_position.ComputeContainerNode()).
> 
> Original change's description:
> > Find-in-page per block using FindBuffer
> >
> > This CL introduces FindBuffer, which Find-in-page will use to find
> > matches instead of using FindPlainText that uses TextIterator.
> >
> > Since find-in-page matches can't span multiple blocks, we are using
> > that fact to know where we can separate the text in the document.
> > FindBuffer collects text one block at a time, and then finds the match
> > in that block. We trigger FindBuffer text collection & finding many
> > times until it goes through everything. FindInPage uses TextSearcherICU
> > to find text, and uses flat-tree traversal, ComputedStyle, and
> > LayoutObject + NGOffsetMapping, whereas the previous way was to use
> > TextIterator.
> >
> > This CL also changes the way we handle newlines - newlines will be
> > saved as object replacement character/whitespaces for Find-in-page
> > purposes. Some tests that rely on newlines are deleted as a result.
> > Note:
> > This change is not user-visible because it is not possible to do a
> > Find-in-page from the UI with newlines.
> >
> > See doc for details:
> > https://docs.google.com/document/d/1D5q7ZMrLPfilXnIGI3SOL0mwdiQC7SQEtGWk-9ysbMg/edit?usp=sharing
> >
> > Bug: 868227
> > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
> > Change-Id: Id5dd7e6c428f59442d02ec88ea0cec6eb4e64c22
> > Reviewed-on: https://chromium-review.googlesource.com/c/1282425
> > Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
> > Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#616701}
> 
> TBR=ellyjones@chromium.org,yosin@chromium.org,yoichio@chromium.org,kojii@chromium.org,xiaochengh@chromium.org,rakina@chromium.org
> 
> Change-Id: Ic06a33d1c630bb4423fc0963bc55b6af5e8ef865
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 868227
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
> Reviewed-on: https://chromium-review.googlesource.com/c/1378407
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Reviewed-by: Hongchan Choi <hongchan@chromium.org>
> Commit-Queue: Hongchan Choi <hongchan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#616757}

TBR=ellyjones@chromium.org,yosin@chromium.org,yoichio@chromium.org,kojii@chromium.org,hongchan@chromium.org,xiaochengh@chromium.org,rakina@chromium.org

Change-Id: Ib48618ecfb9a0daa55cd0bd86bda4744a324d5cf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 868227
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/c/1379086
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616898}
[modify] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc
[modify] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/chrome/test/data/find_in_page/LongFind.txt
[modify] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/BUILD.gn
[add] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/finder/find_buffer.cc
[add] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/finder/find_buffer.h
[add] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc
[modify] https://crrev.com/0be1119f808d95d053e7368aa3352b7de29ecc87/third_party/blink/renderer/core/editing/finder/find_task_controller.cc

Comment 8 by bugdroid, Jan 25

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09bd5e1f27bd8bb1be3564fa568e87bc0a625f3f

commit 09bd5e1f27bd8bb1be3564fa568e87bc0a625f3f
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Fri Jan 25 06:21:11 2019

Make GetMappingUnitsForDOMRange clamp partially out-of-range units

GetMappingUnitsForDOMRange might return units that have partially out
of range, this change makes it modify those units so that all of them
are inside the range given.

Bug: 868227
Change-Id: I3223c27238630b4bbd15cdb6c269b543ef3d3f1f
Reviewed-on: https://chromium-review.googlesource.com/c/1436464
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626011}

Comment 9 by bugdroid, Jan 25

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9ed6c228fffe3831825b107e21bda18619c0d77

commit d9ed6c228fffe3831825b107e21bda18619c0d77
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Fri Jan 25 10:26:51 2019

Allow FindBuffer to find text bounded by a range

This change allows FindBuffer to search for find-in-page matches that
are located inside a range of [start_position, end_position). This will
allow FindBuffer to be used by Editor::FindRangeOfString in the future,
removing that function's usage of TextIterator.

Bug: 868227
Change-Id: Id5b8c04ffa1995b9ee614f58a90901a5cf4075a7
Reviewed-on: https://chromium-review.googlesource.com/c/1424677
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626040}

Comment 10 by bugdroid, Jan 31

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/057b928aa0895165995b4a9f7029a80d3397a397

commit 057b928aa0895165995b4a9f7029a80d3397a397
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Thu Jan 31 08:57:58 2019

Make FindBuffer::FindMatches use blink::FindOptions

Previously FindBuffer uses mojo's FindOptions that doesn't have as many
options as blink::FindOptions. Instead of changing the mojo struct we
resort back to use blink::FindOptions because the options we want to add
are already there and are not related to browser-renderer IPCs.

Bug: 868227
Change-Id: I4371abbbf6b729e653eddfe68f9fd0023b1c01d1
Reviewed-on: https://chromium-review.googlesource.com/c/1446253
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627867}
[modify] https://crrev.com/057b928aa0895165995b4a9f7029a80d3397a397/third_party/blink/renderer/core/editing/finder/find_buffer.cc
[modify] https://crrev.com/057b928aa0895165995b4a9f7029a80d3397a397/third_party/blink/renderer/core/editing/finder/find_buffer.h
[modify] https://crrev.com/057b928aa0895165995b4a9f7029a80d3397a397/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc
[modify] https://crrev.com/057b928aa0895165995b4a9f7029a80d3397a397/third_party/blink/renderer/core/editing/finder/find_task_controller.cc

Comment 11 by bugdroid, Jan 31

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e93205293877d976f513887a6742207de048f442

commit e93205293877d976f513887a6742207de048f442
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Thu Jan 31 09:03:39 2019

Make FindBuffer handle Japanese kana texts correctly

Previously FindBuffer doesn't do special handling of kana characters
like SearchBuffer did, resulting in incorect matching of characters that
aren't supposed to be matched, like ひand び.

Bug: 868227
Change-Id: Id9c7bb7fc38b9256ca6953057677204c0a7063bf
Reviewed-on: https://chromium-review.googlesource.com/c/1444896
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627869}
[modify] https://crrev.com/e93205293877d976f513887a6742207de048f442/third_party/blink/renderer/core/editing/finder/find_buffer.cc
[modify] https://crrev.com/e93205293877d976f513887a6742207de048f442/third_party/blink/renderer/core/editing/finder/find_buffer.h
[modify] https://crrev.com/e93205293877d976f513887a6742207de048f442/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc
[modify] https://crrev.com/e93205293877d976f513887a6742207de048f442/third_party/blink/renderer/core/editing/iterators/text_searcher_icu.cc
[modify] https://crrev.com/e93205293877d976f513887a6742207de048f442/third_party/blink/renderer/core/editing/iterators/text_searcher_icu.h

Comment 12 by bugdroid, Feb 1

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14eab71d62d4e3fadb2cf823e4bdc5e5bca0bd6d

commit 14eab71d62d4e3fadb2cf823e4bdc5e5bca0bd6d
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Fri Feb 01 09:19:00 2019

Support whole-word search in FindBuffer

This CL makes FindBuffer able to filter out non-whole-word find results
if needed. So on the text "foo foobar", there is only 1 match for "foo"
and 0 match for "bar".

Bug: 868227
Change-Id: Iaf31fb71c4b345a71061e6957ecd60f302de930e
Reviewed-on: https://chromium-review.googlesource.com/c/1445354
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628280}
[modify] https://crrev.com/14eab71d62d4e3fadb2cf823e4bdc5e5bca0bd6d/third_party/blink/renderer/core/editing/finder/find_buffer.cc
[modify] https://crrev.com/14eab71d62d4e3fadb2cf823e4bdc5e5bca0bd6d/third_party/blink/renderer/core/editing/finder/find_buffer.h
[modify] https://crrev.com/14eab71d62d4e3fadb2cf823e4bdc5e5bca0bd6d/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc
[modify] https://crrev.com/14eab71d62d4e3fadb2cf823e4bdc5e5bca0bd6d/third_party/blink/renderer/core/editing/iterators/search_buffer.cc
[modify] https://crrev.com/14eab71d62d4e3fadb2cf823e4bdc5e5bca0bd6d/third_party/blink/renderer/core/editing/iterators/text_searcher_icu.cc
[modify] https://crrev.com/14eab71d62d4e3fadb2cf823e4bdc5e5bca0bd6d/third_party/blink/renderer/core/editing/iterators/text_searcher_icu.h
[modify] https://crrev.com/14eab71d62d4e3fadb2cf823e4bdc5e5bca0bd6d/third_party/blink/renderer/core/editing/iterators/text_searcher_icu_test.cc

Comment 13 by bugdroid, Feb 4

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5a58d19b1981c3abd9078efa1bd2e96fd910275

commit e5a58d19b1981c3abd9078efa1bd2e96fd910275
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Mon Feb 04 04:28:29 2019

Make Editor::FindString use flat-tree FindRangeOfString

We're going to remove DOM tree version of FindRangeOfString soon.

Bug: 868227
Change-Id: I42a9d2a417efde646ee20b5d2825a24765d3369e
Reviewed-on: https://chromium-review.googlesource.com/c/1451500
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628654}
[modify] https://crrev.com/e5a58d19b1981c3abd9078efa1bd2e96fd910275/third_party/blink/renderer/core/editing/editor.cc

Comment 14 by bugdroid, Feb 4

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59cc05386e8d1d5bbc911ccc464092f1cff873ca

commit 59cc05386e8d1d5bbc911ccc464092f1cff873ca
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Mon Feb 04 04:42:20 2019

Detemplatize FindRangeOfStringAlgorithm() and FindStringBetweenPositions()

Removing the DOM-tree version in favor of the flat-tree version.

Bug: 868227
Change-Id: Ia64f35117d04bfd9f7ddabc303c416e320225dd3
Reviewed-on: https://chromium-review.googlesource.com/c/1451481
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628656}
[modify] https://crrev.com/59cc05386e8d1d5bbc911ccc464092f1cff873ca/third_party/blink/renderer/core/editing/editor.cc
[modify] https://crrev.com/59cc05386e8d1d5bbc911ccc464092f1cff873ca/third_party/blink/renderer/core/editing/editor.h

Comment 15 by bugdroid, Feb 4

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1cf15c7f5ed50ba69cba650092a73d619199f9a7

commit 1cf15c7f5ed50ba69cba650092a73d619199f9a7
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Mon Feb 04 08:07:15 2019

Make FindStringBetweenPositions use FindBuffer and activate invisible matches

Previously FindStringBetweenPositions uses TextIterator which doesn't
support matching of invisible nodes. This function is used by find-next
but not counting total FIP matches, which uses FindBuffer already.
Thus, the total count of FIP matches is correct (counting invisible
nodes), but when we try to navigate to the match through find-next/prev
it will skip those matches.

This CL modifies FindStringBetweenPositions to use FindBuffer that now
supports finding a match within a range.
This CL also dispatches the "activateinvisible" event to active matches
that are invisible when navigated-to through find-next/prev, because
otherwise it couldn't highlight the range even when it has the range.

Bug: 868227, 873057
Change-Id: Ib9475318bcb38ec1d97e3b029a7eb25495aed060
Reviewed-on: https://chromium-review.googlesource.com/c/1451697
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628683}
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/renderer/core/editing/editor.cc
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/renderer/core/editing/finder/find_buffer.cc
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/renderer/core/editing/finder/find_buffer.h
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/renderer/core/editing/finder/find_buffer_test.cc
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/renderer/core/editing/finder/text_finder.cc
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/renderer/core/invisible_dom/find_invisible_test.cc
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/renderer/core/invisible_dom/invisible_dom.cc
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/renderer/core/invisible_dom/invisible_dom.h
[delete] https://crrev.com/c6bb24c14f1b19f356f75ea8a30a24cbd0a41ae1/third_party/blink/web_tests/editing/execCommand/6355786.html
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/web_tests/editing/text-iterator/findString-expected.txt
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/web_tests/editing/text-iterator/findString.html
[modify] https://crrev.com/1cf15c7f5ed50ba69cba650092a73d619199f9a7/third_party/blink/web_tests/fast/text/selection/find-hidden-text.html

Sign in to add a comment