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

Issue 156919 link

Starred by 6 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Regression



Sign in to add a comment

[Regression] Issue with count in find-in-page

Reported by athigle@chromium.org, Oct 19 2012

Issue description

Chrome Version       : 24.0.1302.0
URLs (if applicable) :
Other browsers tested:
Add OK or FAIL after other browsers where you have tested this issue:
Safari 5:
Firefox 4.x:
IE 7/8/9:

What steps will reproduce the problem?
1. Launch Chrome and go to http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/branches/1180/src&range=143330:146252&mode=html
2. Search for "printpreview"

What is the expected result?
Since there is only one word containing phrase, the count should be one.

What happens instead?
It shows of "1 of 2", but doesn't navigate to the second appearance, possibly counting itself twice?

Please provide any additional information below. Attach a screenshot if
possible.
 
Bisected between 23.0.1241.0 and 23.0.1271.0 on Windows 7

You are probably looking for a change made after 157309 (known good), but no later than 157317 (first known bad).
WEBKIT CHANGELOG URL:
  http://trac.webkit.org/log/trunk/?rev=128849&stop_rev=128773&verbose=on&limit=
10000
CHANGELOG URL:
  http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog.html?url=/tru
nk/src&range=157309%3A157317
This can be bisected further. please narrow the regression window.
There aren't any Chromium builds available between 157309 and 157317 to bisect further.
Labels: Mstone-23
sorry. I misread the bisect info. There is a webkit merge in that window. May be that is the culprit.

Comment 5 by kareng@google.com, Oct 19 2012

Owner: abarth@chromium.org
Status: Assigned
adam could it be https://bugs.webkit.org/show_bug.cgi?id=96402?

http://trac.webkit.org/changeset/128784

Comment 6 by kareng@google.com, Oct 19 2012

Cc: abarth@chromium.org
Owner: leandrogracia@chromium.org

Comment 7 by abarth@chromium.org, Oct 19 2012

Yes.  That's very possibly the problem.

Comment 8 by kareng@google.com, Oct 19 2012

fixing bug # ‚Äčhttps://bugs.webkit.org/show_bug.cgi?id=96402
Cc: vclarke@chromium.org manoranj...@chromium.org pavanv@chromium.org ligim...@chromium.org
Issue 158050 has been merged into this issue.

Comment 10 by kareng@google.com, Oct 26 2012

Labels: -Pri-2 Pri-1 ReleaseBlock-Stable
This issue stil occurs in 24.0.1308.0 canary on Mac.
Cc: leandrogracia@chromium.org
Owner: abarth@chromium.org
There are too many patches to revert to back out leandrogracia's change.  Rolling forward to trunk won't help either because the issue still exists in trunk.  I think our best bet is to fix the issue on trunk and see whether we can merge that back to the branch.

I'll try to do that.
Interesting.  The the page does actually contain the text twice, but one of the times is in an invisible iframe that's hidden using the following technique:

data:text/html,<div>hello</div><iframe style="display:inline; width:0;height:0; border: none" srcdoc="hello"></iframe>

I should check that in M22, but I believe that's a reduced test case.
Yes.  Chrome 22.0.1229.94 says "1 of 1" for that test case, but 24.0.1308.0 canary says "1 of 2".
This patch fixes the issue:

Index: third_party/WebKit/Source/WebKit/chromium/src/WebFrameImpl.cpp
===================================================================
--- third_party/WebKit/Source/WebKit/chromium/src/WebFrameImpl.cpp	(revision 132693)
+++ third_party/WebKit/Source/WebKit/chromium/src/WebFrameImpl.cpp	(working copy)
@@ -2453,7 +2453,7 @@
     // Don't scope if we can't find a frame or a view.
     // The user may have closed the tab/application, so abort.
     // Also ignore detached frames, as many find operations report to the main frame.
-    if (!frame() || !frame()->view() || !frame()->page())
+    if (!frame() || !frame()->view() || !frame()->page() || !hasVisibleContent())
         return false;
 
     ASSERT(frame()->document() && frame()->view());

Patch up for review in https://bugs.webkit.org/show_bug.cgi?id=100604
Labels: WebKit-ID-100604
Labels: -OS-Windows OS-All
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 28 2012

Labels: -WebKit-ID-100604 WebKit-ID-100604-NEW
https://bugs.webkit.org/show_bug.cgi?id=100604
Labels: Merge-Requested
http://trac.webkit.org/changeset/132746
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 28 2012

Labels: -WebKit-ID-100604-NEW WebKit-ID-100604-RESOLVED
https://bugs.webkit.org/show_bug.cgi?id=100604

Comment 22 by kareng@google.com, Oct 29 2012

Labels: -Merge-Requested Merge-Approved
Sorry, http://trac.webkit.org/changeset/132832 is the merge to M23.

Comment 25 by kareng@google.com, Oct 29 2012

Labels: -Merge-Approved Merge-Merged

Comment 26 by kareng@google.com, Oct 29 2012

Status: Fixed
qa verified on canary.
Status: Verified
Verified fixed in 24.0.1311.0 (Official Build 164600) canary on Windows 7
Project Member

Comment 28 by bugdroid1@chromium.org, Mar 9 2013

Labels: -Type-Regression -Area-UI -Feature-FindInPage -Mstone-23 Type-Bug-Regression Cr-UI-Browser-FindInPage Cr-UI M-23

Sign in to add a comment