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

Issue 2220 link

Starred by 63 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

FindNext does not handle dynamic changes to page content

Reported by kurtr...@gmail.com, Sep 13 2008

Issue description

Product Version      : 2.149.29
URLs (if applicable) : http://code.google.com/codejam/results.html
Other browsers tested:
Add OK or FAIL after other browsers where you have tested this issue:
Safari 3:
    Firefox 3: OK
         IE 7: OK

I am having this problem for a page on the SAME tab. Here's how to 
reproduce:

1. Go to http://code.google.com/codejam/results.html
2. Press Ctrl + F and search for 'India'. You'll notice 1 match.
3. Now scroll upwards and click the 'next 250' link at the top of the list.
4. Move focus to the Find box by simply clicking on the Find input box.
5. Press Enter. This should search for 'India' again and should bring up 10 
matches but nothing happens. It still shows '1 of 1' and the up-down 
buttons do nothing. 


 

Comment 1 by finnur@chromium.org, Sep 13 2008

Status: Untriaged
Summary: FindNext doesn't work beyond page 1 of search results
Good reproduction steps. That was missing from  bug 876 .

Thanks!
Labels: -area-unknown Area-Misc
Labels: -Area-Misc Area-BrowserUI

Comment 4 by ben@chromium.org, Oct 22 2008

Labels: Mstone-1.0
Status: Assigned
Labels: -Mstone-1.0 Mstone-1.1
I read this wrong. The problem is on a certain kind of page that reloads content 
without navigating away. I don't see this as 1.0 blocker.

Comment 6 by finnur@chromium.org, Nov 10 2008

I was going to suggest this not be a 1.0 blocker, but you beat me to it. This is actually yet another case of a bug that would be fixed if we automatically hid the Find box on blur. 
Then, when you start interacting with the page (ie. click the "Next Results") the Find box would disappear. My recommendation would be to up the priority on that bug and put it on 
the 1.1 train, if it isn't there already.

In any case, the problem with this bug is that when the user clicks "Next results" the DOM is modified and the tickmark vector is partially/fully invalid. Ojan, Dimitri and I 
brainstormed this problem a bit on Friday and we can think of a number of ways to approach this:

1) We can register for DOM change events and for each removed node, we could evict its descendants from the tickmark vector and for each added node we could do a search on that 
subtree and add it to the vector. This would have to be done in a smart way so that we don't put a big load on the cpu if the DOM is changing a lot.

2) Another approach is that once we FindNext to an invalid tickmark (one that is no longer a descendant of the body node) we clear the tickmark vector for that frame, redo it and 
restore state in such a way that the first new tickmark found becomes active.

3) We can alleviate some of the symptoms by just skipping over and evicting invalid tickmarks as we find them with FindNext (when we jump to an invalid tickmark we evict that 
tickmark and try the next one until we find a valid one or reach the end of the tickmark vector). 

Approach 3 is quick, dead simple, easy and very low risk, but of course means that you don't get new tickmarks added for the dynamic content that was just added to the page. :/ It 
also doesn't address new nodes added to the DOM, it just removed invalid ones.

Approach 2 isn't very complex and doesn't come with much of a performance risk but feels a bit hacky, especially restoring back the state so that we get the right tickmark active 
after redoing the search in the frame. And, of course, the new tickmarks wouldn't appear until you hit an invalid one. I took a stab at this and have it working in my tree, except 
that if you FindNext from tickmark A (valid) to tickmark B (invalid), the new tickmarks appear but tickmark A is still the active one after doing FindNext. Another FindNext moves 
active tickmark status to B. :/

Approach 1 is the most powerful and promising, but also the most complex approach and probably will take time to get right. Definately not worth the risk for 1.0.


My new and improved Find-in-page handles this case a lot better. 

When you click the Next link the table is modified in place (no navigation occurs). 
When you issue FindNext after that you can jump to the entries that were just added. 
The newly added words are not highlighted (except for the active one) and the total 
match count in the find box is not updated, but this is at least a step in the right 
direction.

Comment 8 by kurtr...@gmail.com, Dec 6 2008

I think there is now a similar bug for normal pages too.
Steps:
1) Go to http://www.last.fm/user/kurtrips/charts?rangetype=overall&subtype=artists
2) Press Ctrl + F. Type 'dea'. You'll notice 6 results.
3) Now go to address bar and refresh. You'll notice that Find box remains there.
4) After the refresh, bring focus to the Find box and press Enter. Nothing happens 
now.

I think in previous releases, the Find box used to disappear on page refresh.
Labels: Mstone-2.0

Comment 10 by prog...@gmail.com, Feb 3 2009

the whole find in page feature needs work
there should be a way to pin it (and have the search string "activated" on new pages 
automatically)

it should also have Google Toolbar's way of clicking one word as a "jump to" feature 
, it should support search operators ("", + etc.)

it's Google's browser... it must have a decent search
Finnur, should this be a 2.0 stable release blocker? 
Probably not.
Blockedon: 6759
And, what's more: this issue is blocked on issue 6579, which is marked as MStone-2.1.
Either we up the priority on that bug, or decrease the priority on this bug.
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=13028 

------------------------------------------------------------------------
r13028 | finnur@chromium.org | 2009-04-02 13:07:15 -0700 (Thu, 02 Apr 2009) | 5 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/find_bar_controller.cc?r1=13028&r2=13027
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/web_contents.h?r1=13028&r2=13027

 Bug 2220  lists two cases where the inactive highlighting doesn't reappear on FindNext. This is a simple fix for one of those cases (user presses Refresh after Find).

BUG= 2220 
TEST=Open google.com, press Find, press e, press Refresh, press F3 (or FindNext button in UI). Then make sure the inactive matches are highlighted (not just the active match).
Review URL: http://codereview.chromium.org/56193
------------------------------------------------------------------------

The autolink above (comment 13) is incorrect. I meant  issue 6759  when I said 6579.

Comment 16 by jon@chromium.org, Apr 3 2009

Labels: JonMoved Mstone-2.1
Moving from milestone 2 to milestone 2.1.
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=13561 

------------------------------------------------------------------------
r13561 | laforge@chromium.org | 2009-04-11 12:54:38 -0700 (Sat, 11 Apr 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/172/src/chrome/browser/find_bar_controller.cc?r1=13561&r2=13560
   M http://src.chromium.org/viewvc/chrome/branches/172/src/chrome/browser/tab_contents/web_contents.h?r1=13561&r2=13560

Merge 13028 -  Bug 2220  lists two cases where the inactive highlighting doesn't reappear on FindNext. This is a simple fix for one of those cases (user presses Refresh after Find).

BUG= 2220 
TEST=Open google.com, press Find, press e, press Refresh, press F3 (or FindNext button in UI). Then make sure the inactive matches are highlighted (not just the active match).
Review URL: http://codereview.chromium.org/56193
TBR=finnur@chromium.org
Review URL: http://codereview.chromium.org/67068
------------------------------------------------------------------------

Labels: -Mstone-2.1 Mstone-2.2
Labels: -jonmoved

Comment 20 by jon@chromium.org, Jun 15 2009

Labels: Mstone-3
Status: Untriaged
Moving to mstone 3 from an older milestone.  Need to triage.
Labels: -Mstone-3 Mstone-X Fixit
Status: Available
 Issue 17995  has been merged into this issue.
new repro steps from  issue 17995  using Picasa Web Albums

1. enter http://picasaweb.google.com/lh/featured?feat=featured_all#
2. CTRL+F
3. type "by" for example or... "2009"
4. select a second Picasa search results page while the find text box is 
still opened 

a) number of occurences is not updated
b) pressing CTRL+F re-highlights the find text box (leaving the user with the 
impression that the results have been updated, but they are not)


Labels: Feature-FindInPage

Comment 25 by oritm@chromium.org, Dec 18 2009

Labels: -Area-BrowserUI Area-UI-Features
Area-UI-Features label replaces Area-BrowserUI label
Labels: Area-UI
Labels: -Area-UI-Features
I think this belongs here, since it still involves erratic behaviour with find-in-
page within dynamically changing content, but I've noticed a slight variant on the 
behaviour when the originally "found" instances still appear on the page.

For example, load http://twitter.com/google

1. Ctrl+F and type something that can be found in every tweet, like "via"
2. Find previous to take you to the last instance
3. Hit "more" to load more tweets
4. Hit the arrow to find next

What seems to happen is that Chrome scrolls to the next tweet, which resides in the 
newly revealed list, but does not update the number of found strings, and doesn't 
highlight them. The number identifying the found string is cycled back to 1, and 
keeps advancing, and scrolling appropriately, but never highlighting.
Labels: -Fixit bulkmove TaskForce-Fixit
Product Version      : 2.149.29
URLs (if applicable) : http://code.google.com/codejam/results.html
Other browsers tested:
Add OK or FAIL after other browsers where you have tested this issue:
Safari 3:
    Firefox 3: OK
         IE 7: OK

I am having this problem for a page on the SAME tab. Here's how to 
reproduce:

1. Go to http://code.google.com/codejam/results.html
2. Press Ctrl + F and search for 'India'. You'll notice 1 match.
3. Now scroll upwards and click the 'next 250' link at the top of the list.
4. Move focus to the Find box by simply clicking on the Find input box.
5. Press Enter. This should search for 'India' again and should bring up 10 
matches but nothing happens. It still shows '1 of 1' and the up-down 
buttons do nothing.
Project Member

Comment 30 by bugdroid1@chromium.org, Jun 8 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=88250

------------------------------------------------------------------------
r88250 | zhurunz@google.com | Tue Jun 07 17:29:51 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/o3d/build/common.gypi?r1=88250&r2=88249&pathrev=88250
 M http://src.chromium.org/viewvc/chrome/trunk/o3d/plugin/plugin.gyp?r1=88250&r2=88249&pathrev=88250
 M http://src.chromium.org/viewvc/chrome/trunk/o3d/build/libs.gyp?r1=88250&r2=88249&pathrev=88250

Upstream ChromeOS patch on O3D. (Change Icf3bc91d: http://gerrit.chromium.org/gerrit/#change,2220)
Review URL: http://codereview.chromium.org/7108010
------------------------------------------------------------------------
Labels: Hotlist-PolishFixit
 Issue 168123  has been merged into this issue.
Project Member

Comment 33 by bugdroid1@chromium.org, Mar 10 2013

Blockedon: -chromium:6759
Labels: -Feature-FindInPage -Area-UI Cr-UI Cr-UI-Browser-FindInPage

Comment 34 Deleted

I can't believe this is here for more than 6 years and it's affecting anybody anywhere.

1. Just do a simple search @ google.com
2. search for sth.
3. go to next page of search results
4. press F3

Workaround: Press Esc before executing step 4
Pretty embarrassing IMHO...
Just so we're clear on what you're saying, Christopher on the off
chance I misunderstood your post.

You're demonstrating that you're not affected by the bug by doing a
Google search for a work around, encountering the problem and then
employing a work around?  I was talking with our CTO the other day and
at a previous place he was employed the solution to "the server
crashes every day" was "reboot the server every day".  Yeah, I guess
you could do that; but maybe fixing the problem is a better solution.

I'm just saying a lot of Bugzilla entries around the world could be
closed out by "Google a workaround and then employ every time said bug
appears".  Personally, I think my users would still consider it a bug.
Ticket closed or not.
No, you got me wrong. I think this should have been fixed a LONG time ago and it's embarassing that it's still here, cause it's affecting basically anyone of us.
I mentioned steps that most people do every day (Google is the nr 1 search engine in Europe after all) and just amended a temporary workaround for those who are desperately waiting for this to get fixed just like me.
Ah.  I agree.  My apologies; I completely misread your post. :)
Project Member

Comment 39 by bugdroid1@chromium.org, Feb 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22cbbba006181412d51841e6bf3a7b53d9b653df

commit 22cbbba006181412d51841e6bf3a7b53d9b653df
Author: dvadym <dvadym@chromium.org>
Date: Tue Feb 23 15:49:14 2016

Restart search in page when new text is found.

This patch is a partial solution to the problem, when DOM was changed and
Find Next In Page fails to show correct results that corresponding to the
current DOM.

For context, some details how Find in Page works:
1. When the user initializes new search RenderFrameImpl::OnFind is called, in
order not to block UI it quickly returns the first result and schedules new full
search task (it's called scoping request in code).

2. The full search task runs and adds all occurrences of the searched text as
markers in DocumentMarkerController. These markers are  presented to the user as
search results.

3. When the user starts FindNext in browser, again RenderFrameImpl::OnFind is
called, it runs search from the place of previous search result and in
TextFinder it's called DocumentMarkerController::setMarkersActive to set the
current result. No new markers to DocumentMarkerController are added.

Current FindNext implementation (TextFinder) uses current DOM and gives correct
results, the problem is that DocumentMarkerController::setMarkersActive silently
fails to add marker when it receives a search result in content that was added
after full search in 2.

The proposed solution in this patch:
1. When DocumentMarkerController::setMarkersActive fails to find marker that
corresponds to a current FindNext result (that means that it's new part of DOM,
that was loaded after the initial full search), it adds this result range as
active marker and returns false to TextFinder.

2. TextFinder receives information that the new text found, it stores restore
point to the following full search (so search will continue from the same
place). And returns to RenderFrameImpl activeNow=false.

3. RenderFrameImpl receives that activeNow=false restarts full search (in a
separate task, the same as with original search).

This patch doesn't solve all problems, namely
1. In order to see new results the user should use FindNext till new results are
found.

2. If something is removed from DOM, new search is not restarted (it's hard to
find out about this from DocumentMarkerController), so the user will see
incorrect number of results, but the user will still see correct highlighted
search results in a page (the same behaviour as now).

but it makes problems much less severe.

BUG= 2220 

Review URL: https://codereview.chromium.org/1605863002

Cr-Commit-Position: refs/heads/master@{#376994}

[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.h
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/Source/core/editing/markers/DocumentMarkerControllerTest.cpp
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/Source/web/TextFinder.cpp
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/Source/web/TextFinder.h
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/Source/web/tests/TextFinderTest.cpp
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[modify] https://crrev.com/22cbbba006181412d51841e6bf3a7b53d9b653df/third_party/WebKit/public/web/WebLocalFrame.h

Cc: ranjitkan@chromium.org
 Issue 346271  has been merged into this issue.
The patch from #39 doesn't solve the problem fully, but it fixes the most annoying cases. 

It works in the following way: as soon as by pressing Find Next a new match in DOM (i.e a match that was not present on initial search) is found, full search is restarted and all matches are highlighted according to the current DOM and a number of matches in a search bar is updated.

It's not full solution, because:
  1.In order to restart search it's needed to come by pressing Find Next to a new match. Restarting is not triggered on event of updating of DOM or for example on the first FindNext after DOM update.
  2.It doesn't handle removals of matches from DOM and the search is not restarted on removals and an old number of matches are shown in Search Bar (of course removed matches are not highlighted, so it's not critical).

 
Summary: FindNext does handle dynamic changes to page content (was: FindNext doesn't work beyond page 1 of search results)
Cc: ssamanoori@chromium.org adamk@chromium.org pbos@chromium.org esprehn@chromium.org justincohen@chromium.org nyerramilli@chromium.org smokana@chromium.org yaar@chromium.org rafaelw@chromium.org
 Issue 177262  has been merged into this issue.
Issue 583412 has been merged into this issue.
Project Member

Comment 45 by bugdroid1@chromium.org, Sep 23 2016

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

commit ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1
Author: paulmeyer <paulmeyer@chromium.org>
Date: Fri Sep 23 20:25:34 2016

Handling new frames and frame navigations with find-in-page during a find session.

This patch implements new find-in-page functionality that allows for frames that are either newly added or navigated during a find session to be automatically searched so that matches in their new content are automatically highlighted and included in the results shown in the find bar.

Also, there is a fix included in this patch to prevent the find-in-page bar from closing when a subframe navigates (the find session should only end if the main frame navigates).

Design doc: https://docs.google.com/document/d/1r4F19FIKg4zPJCaSyJ9-T0sgFbxlCGKL3FjqQEAKegg/view

BUG= 2220 , 621660 

Review-Url: https://codereview.chromium.org/2236403004
Cr-Commit-Position: refs/heads/master@{#420715}

[modify] https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1/chrome/browser/ui/find_bar/find_bar_controller.cc
[modify] https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1/content/browser/find_request_manager.cc
[modify] https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1/content/browser/find_request_manager.h
[modify] https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1/content/browser/find_request_manager_browsertest.cc
[modify] https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1/content/common/frame_messages.h
[modify] https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1/third_party/WebKit/Source/web/TextFinder.cpp
[modify] https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1/third_party/WebKit/Source/web/TextFinder.h
[modify] https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1/third_party/WebKit/public/web/WebFindOptions.h

Project Member

Comment 46 by bugdroid1@chromium.org, Sep 23 2016

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

commit cab0f2e9bc64a1caf21877b36972a30212c685e6
Author: gcasto <gcasto@chromium.org>
Date: Fri Sep 23 23:16:05 2016

Revert of Handling new frames and frame navigations with find-in-page during a find session. (patchset #2 id:120001 of https://codereview.chromium.org/2236403004/ )

Reason for revert:
This seems to be failing pretty consistently on some bots (e.g. https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29)

Original issue's description:
> Handling new frames and frame navigations with find-in-page during a find session.
>
> This patch implements new find-in-page functionality that allows for frames that are either newly added or navigated during a find session to be automatically searched so that matches in their new content are automatically highlighted and included in the results shown in the find bar.
>
> Also, there is a fix included in this patch to prevent the find-in-page bar from closing when a subframe navigates (the find session should only end if the main frame navigates).
>
> Design doc: https://docs.google.com/document/d/1r4F19FIKg4zPJCaSyJ9-T0sgFbxlCGKL3FjqQEAKegg/view
>
> BUG= 2220 , 621660 
>
> Committed: https://crrev.com/ec9c7f30eb10ab3fc9ab6707f7b736446ab89aa1
> Cr-Commit-Position: refs/heads/master@{#420715}

TBR=dcheng@chromium.org,pkasting@chromium.org,kenrb@chromium.org,paulmeyer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 2220 , 621660 

Review-Url: https://codereview.chromium.org/2363993003
Cr-Commit-Position: refs/heads/master@{#420774}

[modify] https://crrev.com/cab0f2e9bc64a1caf21877b36972a30212c685e6/chrome/browser/ui/find_bar/find_bar_controller.cc
[modify] https://crrev.com/cab0f2e9bc64a1caf21877b36972a30212c685e6/content/browser/find_request_manager.cc
[modify] https://crrev.com/cab0f2e9bc64a1caf21877b36972a30212c685e6/content/browser/find_request_manager.h
[modify] https://crrev.com/cab0f2e9bc64a1caf21877b36972a30212c685e6/content/browser/find_request_manager_browsertest.cc
[modify] https://crrev.com/cab0f2e9bc64a1caf21877b36972a30212c685e6/content/common/frame_messages.h
[modify] https://crrev.com/cab0f2e9bc64a1caf21877b36972a30212c685e6/third_party/WebKit/Source/web/TextFinder.cpp
[modify] https://crrev.com/cab0f2e9bc64a1caf21877b36972a30212c685e6/third_party/WebKit/Source/web/TextFinder.h
[modify] https://crrev.com/cab0f2e9bc64a1caf21877b36972a30212c685e6/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/cab0f2e9bc64a1caf21877b36972a30212c685e6/third_party/WebKit/public/web/WebFindOptions.h

Project Member

Comment 47 by bugdroid1@chromium.org, Sep 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ac612d592cd42355a432c8d307051d299d79768

commit 3ac612d592cd42355a432c8d307051d299d79768
Author: paulmeyer <paulmeyer@chromium.org>
Date: Fri Sep 30 19:21:06 2016

Handling new frames and frame navigations with find-in-page during a find
session.

This patch implements new find-in-page functionality that allows for frames
that are either newly added or navigated during a find session to be
automatically searched so that matches in their new content are
automatically highlighted and included in the results shown in the find bar.

Also, there is a fix included in this patch to prevent the find-in-page bar
from closing when a subframe navigates (the find session should only end if
the main frame navigates).

Design doc: https://docs.google.com/document/d/1r4F19FIKg4zPJCaSyJ9-T0sgFbxlCGKL3FjqQEAKegg/view

This is a reland of: https://codereview.chromium.org/2236403004/

BUG= 2220 ,  621660 

TBR=dcheng@chromium.org,pkasting@chromium.org

Review-Url: https://codereview.chromium.org/2384483004
Cr-Commit-Position: refs/heads/master@{#422176}

[modify] https://crrev.com/3ac612d592cd42355a432c8d307051d299d79768/chrome/browser/ui/find_bar/find_bar_controller.cc
[modify] https://crrev.com/3ac612d592cd42355a432c8d307051d299d79768/content/browser/find_request_manager.cc
[modify] https://crrev.com/3ac612d592cd42355a432c8d307051d299d79768/content/browser/find_request_manager.h
[modify] https://crrev.com/3ac612d592cd42355a432c8d307051d299d79768/content/browser/find_request_manager_browsertest.cc
[modify] https://crrev.com/3ac612d592cd42355a432c8d307051d299d79768/content/common/frame_messages.h
[modify] https://crrev.com/3ac612d592cd42355a432c8d307051d299d79768/third_party/WebKit/Source/web/TextFinder.cpp
[modify] https://crrev.com/3ac612d592cd42355a432c8d307051d299d79768/third_party/WebKit/Source/web/TextFinder.h
[modify] https://crrev.com/3ac612d592cd42355a432c8d307051d299d79768/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/3ac612d592cd42355a432c8d307051d299d79768/third_party/WebKit/public/web/WebFindOptions.h

I have come here from  Issue 536632  via  Issue 177262  (which was merged here), and have tried 55.0.2877.0 canary. This version should contain the commits from both #39 (which is the more relevant one, I guess) and #47:
https://chromium.googlesource.com/chromium/src.git/+log/55.0.2877.0

Yet the bug described in  Issue 536632  (both #0 and #1) still exists.

I don't have access to the design doc, so I wonder whether I might be missing something to active this new feature. chrome://flags/ does not show anything relevant.
mr.berker@:

There isn't anything extra needed to activate this feature. The patch in #47 allows matches in new or navigated frames to be found, while the patch in #37 should allow any matches in existing frames with content that changes to be found (that is the one relevant to your issue).

After trying out some examples (including yours from  issue 536632 ), it looks like the new mechanism from #37 only works if there was already at least one match before the new ones were added. This is because when there are already NO matches found in a frame, the frame is skipped automatically in the next search, meaning new matches will never be found. Completely removing this optimization would fix your problem, but I'm not sure if that's the best thing to do (it's usually good to avoid searching frames repeatedly when it is already known that it contains no matches).


Correction: when I said #37, I meant #39.
Thanks for the explanation. I agree it may be a relevant decrease of efforts "to avoid searching frames repeatedly when it is already known that it contains no matches". Yet I wonder whether there is any good reason to believe that a frame that has had no matches before is less likely to have NEW matches in a subsequent search than a frame that HAS had matches before.

I haven't checked your implementations in #47 and #39, but the most obvious optimization to me (which you are probably doing) is to only re-search frames that have been navigated and/or changed content, and in that case, I would guess that there should be no difference in expecting new matches.
Owner: paulmeyer@chromium.org
Status: Started (was: Available)
I'll try to get this sorted out once and for all.
 Issue 668489  has been merged into this issue.
Summary: FindNext does not handle dynamic changes to page content (was: FindNext does handle dynamic changes to page content)
Cc: jmukthavaram@chromium.org paulmeyer@chromium.org
Issue 655797 has been merged into this issue.

Comment 56 Deleted

Cc: -esprehn@chromium.org
Status: Fixed (was: Started)
Although there may still be specific cases where dynamic content is not picked up, I believe that this bug is too general to just be kept open forever, when the most general cases (as well as other less general cases, such as frame/GuestView removal/navigation/addition, etc.) have already been fixed.

For any remaining specific cases, I think it would be useful to report those as individual bugs that can be considered further.

Comment 59 by pbos@chromium.org, Aug 29 2017

My original bug that got duped against this eventually is definitely fixed. Thanks!
My numerous bugs that were duped here are partially fixed (added content is found after update) - thanks! However, removed content is still "found" after update. I opened  Issue 760450  with details.
Still not fixed. Still reproducible for me. Quite an annoying bug having to change the text in the find field before it notices the content change.

Sign in to add a comment