Incorrect search result count
Reported by
mekishiz...@gmail.com,
Sep 30
|
||||||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.22 Safari/537.36 Steps to reproduce the problem: 1. Open https://blog.cloudflare.com/esni/ 2. Search for "rfc" 3. Observe the count is 0/1 https://i.imgur.com/BPgUdvD.png I noticed this first using Brave Beta (70.0.3538.22) but it's reproducible on the current Canary as well (71.0.3566.0). What is the expected behavior? The count should be 1/1. What went wrong? The count is not correct. It corrects itself after going to the next result (cmd+g). Did this work before? Yes 69.0.3497.100 Chrome version: 70.0.3538.22 Channel: n/a OS Version: OS X 10.13.6 Flash Version:
,
Sep 30
Confirmed in latest Canary.
,
Oct 1
Able to reproduce the issue on reported chrome version 70.0.3538.22 and on the latest canary 71.0.3566.0 using Mac 10.13.1, Ubuntu 14.04 and Windows 10. Bisect Information: ------------------- Good Build: 70.0.3525.0 Bad Build: 70.0.3526.0 You are probably looking for a change made after 584033 (known good), but no later than 584034 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/7f46feee49916ee0a98a7092e495ceebbf135b96..d4ce97f15ace5e4431e8702fdce563fe67299b2f Suspecting: https://chromium.googlesource.com/chromium/src/+/d4ce97f15ace5e4431e8702fdce563fe67299b2f Review URL: https://chromium-review.googlesource.com/1152710 @Rakina Zata Amni: Please help in assigning it to the right owner if this is not related to your change. Adding RB-Stable as this seems to be a recent regression. Thanks!
,
Oct 1
[bulk edit] - This issue is marked as a stable blocker for M70. We are two weeks away from M70 Stable. Please take a look urgently!
,
Oct 4
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b69524e0e1dcf571fe51ab44df6564eeda0bad4 commit 6b69524e0e1dcf571fe51ab44df6564eeda0bad4 Author: Rakina Zata Amni <rakina@chromium.org> Date: Thu Oct 04 08:14:11 2018 Make sure active match is updated correctly in various cases. In cases where there are only one match, active match ordinal might not update properly due to prevention of update of UI. In other cases when Find-in-page is forcefully redone due to DidFinishLoad call, previously found active match is not reactivated correctly. This CL fixes both of them. Bug: 890622 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: Icdb287af7a4cd27bdfb70dba1f53e12bd46d4723 Reviewed-on: https://chromium-review.googlesource.com/c/1256393 Reviewed-by: Hayato Ito <hayato@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/heads/master@{#596543} [modify] https://crrev.com/6b69524e0e1dcf571fe51ab44df6564eeda0bad4/chrome/browser/ui/find_bar/find_bar_controller.cc [modify] https://crrev.com/6b69524e0e1dcf571fe51ab44df6564eeda0bad4/chrome/browser/ui/find_bar/find_bar_controller.h [modify] https://crrev.com/6b69524e0e1dcf571fe51ab44df6564eeda0bad4/third_party/blink/renderer/core/editing/finder/text_finder.cc [modify] https://crrev.com/6b69524e0e1dcf571fe51ab44df6564eeda0bad4/third_party/blink/renderer/core/editing/finder/text_finder.h [modify] https://crrev.com/6b69524e0e1dcf571fe51ab44df6564eeda0bad4/third_party/blink/renderer/core/exported/web_frame_test.cc
,
Oct 4
I think this is fixed by my CL (confirmed locally).
,
Oct 4
[Auto-generated comment by a script] We noticed that this issue is targeted for M-70; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-70 label, otherwise remove Merge-TBD label. Thanks.
,
Oct 4
,
Oct 4
This bug requires manual review: We are only 11 days from stable. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 4
how safe is this merge overall? We are less than 2 weeks away from stable. How critical is this bug?
,
Oct 4
Hmm, I think it's quite safe as it's just affecting Find-in-page, and at worst only affecting cases where find-in-page is done when the page hasn't finished loading yet. However I'm fine if this is going into 71 instead. This bug only affects two cases: when there is only one match for Find-in-page (but even then not always), and when find-in-page is done when the page hasn't finished loading yet.
,
Oct 5
Let's target M71.
,
Oct 5
,
Oct 5
On a second thought, this is impacting user functionality and could be misleading. Have you verified this in canary?
,
Oct 5
,
Oct 5
This bug requires manual review: We are only 10 days from stable. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 6
If "verified" means that the bug is not happening anymore in my build after the fix, yes. Though I've only tested on the sample page and another normal webpage and only on desktop.
,
Oct 8
Approving this merge for M70. Branch:3538. Can you please verify this on a canary build also?
,
Oct 8
chatting with rakina@ offline, this CL still needs still more verification. It's unclear whether the multi-frame scenario is newly broken or if it was existing from before. Removing merge-approved label. Please re-add Merge-Request-70 when it's fully ready and confirmed on canary.
,
Oct 9
The multiple-frames bug is not due to the fix but due to the original regression CL. My fix still fixes the same-frame case, just not when there are multiple frames and the match is in a frame other than the one currently focused on. I found the problem for the multiple-frames case and made a CL that fixed it (confirmed on local build) https://chromium-review.googlesource.com/c/chromium/src/+/1270196, but we think the root cause might be Blink's scheduler, and yosin@ prefers for the scheduler to be fixed instead, and that might not make it to 70. Anyways, I confirmed that the first fix (same-frame case) is working on Canary. Do you think we should also fix the multiple frame case now?
,
Oct 9
This bug requires manual review: We are only 6 days from stable. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 9
Adding altimin@ for the blink scheduler. See #21 Is this a known issue where idle callbacks may not happen?
,
Oct 9
re #21 - let's proceed with the fix for single frame case in M70. And let's leave this bug open for discussion around multi-frame fix/blink scheduler fix. Approving merge for fix in #6 to M70.
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b8b424fc547c72c19a97d5710e9e90a42199798 commit 8b8b424fc547c72c19a97d5710e9e90a42199798 Author: Rakina Zata Amni <rakina@chromium.org> Date: Tue Oct 09 23:05:56 2018 Make sure active match is updated correctly in various cases. In cases where there are only one match, active match ordinal might not update properly due to prevention of update of UI. In other cases when Find-in-page is forcefully redone due to DidFinishLoad call, previously found active match is not reactivated correctly. This CL fixes both of them. Bug: 890622 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: Icdb287af7a4cd27bdfb70dba1f53e12bd46d4723 Reviewed-on: https://chromium-review.googlesource.com/c/1256393 Reviewed-by: Hayato Ito <hayato@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#596543}(cherry picked from commit 6b69524e0e1dcf571fe51ab44df6564eeda0bad4) Reviewed-on: https://chromium-review.googlesource.com/c/1272117 Reviewed-by: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#934} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/8b8b424fc547c72c19a97d5710e9e90a42199798/chrome/browser/ui/find_bar/find_bar_controller.cc [modify] https://crrev.com/8b8b424fc547c72c19a97d5710e9e90a42199798/chrome/browser/ui/find_bar/find_bar_controller.h [modify] https://crrev.com/8b8b424fc547c72c19a97d5710e9e90a42199798/third_party/blink/renderer/core/editing/finder/text_finder.cc [modify] https://crrev.com/8b8b424fc547c72c19a97d5710e9e90a42199798/third_party/blink/renderer/core/editing/finder/text_finder.h [modify] https://crrev.com/8b8b424fc547c72c19a97d5710e9e90a42199798/third_party/blink/renderer/core/exported/web_frame_test.cc
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b8b424fc547c72c19a97d5710e9e90a42199798 Commit: 8b8b424fc547c72c19a97d5710e9e90a42199798 Author: rakina@chromium.org Commiter: abdulsyed@google.com Date: 2018-10-09 23:05:56 +0000 UTC Make sure active match is updated correctly in various cases. In cases where there are only one match, active match ordinal might not update properly due to prevention of update of UI. In other cases when Find-in-page is forcefully redone due to DidFinishLoad call, previously found active match is not reactivated correctly. This CL fixes both of them. Bug: 890622 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: Icdb287af7a4cd27bdfb70dba1f53e12bd46d4723 Reviewed-on: https://chromium-review.googlesource.com/c/1256393 Reviewed-by: Hayato Ito <hayato@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#596543}(cherry picked from commit 6b69524e0e1dcf571fe51ab44df6564eeda0bad4) Reviewed-on: https://chromium-review.googlesource.com/c/1272117 Reviewed-by: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#934} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
,
Oct 9
,
Oct 11
For multiple thread issue, it seems the root cause is IFRAME for https://disqus.com/embed/comments/ which loads contents with XHR and timers after IFRAME's load event. This observation leads us to change FIP architecture: - Detect contents change in LocalFrame and continue search - FIP should not wait completion of all LocalFrame's find task.
,
Oct 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0df2607f9809b189f7b7177df1d19d7cf8429f3c commit 0df2607f9809b189f7b7177df1d19d7cf8429f3c Author: Rakina Zata Amni <rakina@chromium.org> Date: Mon Oct 15 03:58:13 2018 Add deadline to Find-in-page idle tasks In some cases such as multiple frame webpages, idle task might never execute or take a noticably long time to execute. This might lead to incorrect number of find-in-page result and not highlighting matches because the scoping is never trigerred. This CL adds a deadline to scoping idle tasks to ensure all scoping happens in a timely manner. Bug: 890622 , 893465 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: I8ec998d4c2f42377361c1f7b95c1497a80368ef6 Reviewed-on: https://chromium-review.googlesource.com/c/1270196 Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#599562} [modify] https://crrev.com/0df2607f9809b189f7b7177df1d19d7cf8429f3c/third_party/blink/renderer/core/editing/finder/text_finder.cc
,
Oct 15
,
Oct 16
Verified the fix on Mac 10.13.1, Windows-10 and Ubuntu 14.04 using Chrome version #72.0.3582.0 as per the comment #0. Attaching screen cast for reference. Observed the search result as 1/1 Hence, the fix is working as expected. Adding the verified labels. Note: Able to reproduce the issue on chrome version with out fix. Thanks...!!
,
Oct 16
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 16
Pls merge your change to M71 branch 3578 before 1:00 PM PT today, 10/16 so we can pick it up for tomorrow's M71 Dev release. Thank you.
,
Oct 16
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/370d5b4e71b5d71106bbd3f89817df4cce97b299 commit 370d5b4e71b5d71106bbd3f89817df4cce97b299 Author: Rakina Zata Amni <rakina@chromium.org> Date: Tue Oct 16 22:21:08 2018 Add deadline to Find-in-page idle tasks In some cases such as multiple frame webpages, idle task might never execute or take a noticably long time to execute. This might lead to incorrect number of find-in-page result and not highlighting matches because the scoping is never trigerred. This CL adds a deadline to scoping idle tasks to ensure all scoping happens in a timely manner. Bug: 890622 , 893465 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: I8ec998d4c2f42377361c1f7b95c1497a80368ef6 Reviewed-on: https://chromium-review.googlesource.com/c/1270196 Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#599562}(cherry picked from commit 0df2607f9809b189f7b7177df1d19d7cf8429f3c) Reviewed-on: https://chromium-review.googlesource.com/c/1285149 Reviewed-by: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#69} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/370d5b4e71b5d71106bbd3f89817df4cce97b299/third_party/blink/renderer/core/editing/finder/text_finder.cc
,
Oct 23
Verified the fix on Mac 10.13.1, Windows-10 and Ubuntu 14.04 using Chrome version #71.0.3578.20 as per the comment #0. Attaching screen cast for reference. Observed the search result as 1/1 Hence, the fix is working as expected. Adding the verified labels. Note: Able to reproduce the issue on chrome version with out fix. Thanks...!!
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/370d5b4e71b5d71106bbd3f89817df4cce97b299 Commit: 370d5b4e71b5d71106bbd3f89817df4cce97b299 Author: rakina@chromium.org Commiter: rakina@chromium.org Date: 2018-10-16 22:21:08 +0000 UTC Add deadline to Find-in-page idle tasks In some cases such as multiple frame webpages, idle task might never execute or take a noticably long time to execute. This might lead to incorrect number of find-in-page result and not highlighting matches because the scoping is never trigerred. This CL adds a deadline to scoping idle tasks to ensure all scoping happens in a timely manner. Bug: 890622 , 893465 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: I8ec998d4c2f42377361c1f7b95c1497a80368ef6 Reviewed-on: https://chromium-review.googlesource.com/c/1270196 Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#599562}(cherry picked from commit 0df2607f9809b189f7b7177df1d19d7cf8429f3c) Reviewed-on: https://chromium-review.googlesource.com/c/1285149 Reviewed-by: Rakina Zata Amni <rakina@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#69} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
||||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||||
Comment 1 by phanindra.mandapaka@chromium.org
, Sep 30