New issue
Advanced search Search tips

Issue 911611 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Dec 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Search count for the words fails to get updated on deleting a searched text partially in FIP

Project Member Reported by ghuphran@chromium.org, Dec 4

Issue description

App Version: 72.0.3626.0 Canary
iOS Version:  11.4.1, 12.0.1, 12.1 beta 3
Device: iPad Air, iPhone 7

Steps to reproduce:
  1. Launch Google Chrome
  2. Navigate to “https://en.wikipedia.org/wiki/Wihtred_of_Kent” 
  3. Search for any text for e.g,  ‘and Wihtred’
  4. Delete the searched text partially, such that only ‘Wihtred’ remains in the search box

Alternate Steps to Reproduce:

  1. Launch Google Chrome
  2. Navigate to “https://en.wikipedia.org/wiki/Wihtred_of_Kent” 
  3. Search for any text that ends with fullstop, for e.g,  ‘day’
  4. Adding and deleting an extra space after typing in the word, for e.g, add an extra space after the word ‘day’ and delete it
  
Observed results:
Observe that page is populated with words highlighted in yellow, but the counter remains ‘0’.
 
Expected results:
Search count for the words should get updated on deleting a searched text partially in FIP

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes
Bug reproducible after clearing cache and cookies: Yes
Bug reproducible on Chrome Mobile on Android: NA
Bug reproducible on Safari/Firefox: Firefox: NA, Safari: NA
Bug reproducible on current stable build (App Version, iOS Version): No on M70
Bug reproducible on the current beta channel build (App Version, iOS Version): Yes on M71

Type-bug-regression? No 

Link to video/image:
https://drive.google.com/open?id=1vsp40fy5aCGfix6tHT6PZipqCpCOhZ5A

Link to video for M_70
https://drive.google.com/open?id=1AUN2IthuS0wuUoalcrHjdvncSdon3VzV
 
Labels: -Type-Bug Type-Bug-Regression
Sorry, this looks like a regression in M72.

Good Build: 72.0.3624.0
Revision No. - ecf0ae78590173baab05

Bad Build: 72.0.3625.0 beta
Revision No. - 2c59a8a07afb8b113544
Owner: mrsuyi@chromium.org

Comment 3 Deleted

Status: Assigned (was: Untriaged)
Cc: ghuphran@chromium.org
Can this bug be closed as Fixed? or are there any follow up CLs coming?
ghuphran@ to verify after the Fix.

Comment 6 Deleted

Sorry, I wrote the wrong bug number in http://crrev/c/1365610. I'll take a look at this bug soon.
Status: Fixed (was: Assigned)
This bug has been fixed by this CL(http://crrev/c/1370173).
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 17

Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c0428fb7b0941152ce21d0676a0589e170437ea

commit 7c0428fb7b0941152ce21d0676a0589e170437ea
Author: Yi Su <mrsuyi@chromium.org>
Date: Mon Dec 17 19:02:03 2018

Remove highlight delay for FindInPage results out of screen.

In current implementation of find_in_page.js, when user navigates to a
match result out of the screen, the orange highlight will be added to it
with a 250ms delay. In previous version, the callback invoked after
250ms will always add orange highlight to the current selected match
result, and the CL(http://crrev/c/1347363) accidently changed the logic
and created a bug. If user clicks "goNext" fast enough, selected match
result may get highlighted permanently.

However, after deeper investigation, the delayed highlight seems to be
buggy from the begining. Navigating backward to a match result that is
inside the screen will also meets the condition and trigger the delay,
but the 250ms is just too short to be noticed.
Besides, why we need the delay is also mysterious, and here is my guess:
In ancient time the find_in_page.js do two things, first scroll to the
match result and then do the highlight, and somehow UIWebView requires
us to add a delay(see the comments in old version code). Then the
scrolling work moved from Js into OC, so there are two lines about
scrolling that are commented out.

Since UX has also confirmed that delay on highlight is not necessary, it
should be removed now.

Bug:  911611 
Change-Id: If0fef219b33954e243e527f3b50dbe836f753ede
Reviewed-on: https://chromium-review.googlesource.com/c/1365610
Reviewed-by: Peter Lee <pkl@chromium.org>
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614353}(cherry picked from commit 40735d1fe4a48f42b6ec7b71dcaa92c3f04a095f)
Reviewed-on: https://chromium-review.googlesource.com/c/1380534
Reviewed-by: Yi Su <mrsuyi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#398}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/7c0428fb7b0941152ce21d0676a0589e170437ea/ios/chrome/browser/find_in_page/resources/find_in_page.js

Status: Verified (was: Fixed)
Search results are correctly highlighted.

Verified on M72.0.3626.28 beta
iOS: 12.1.2, iPhone7Plus

Sign in to add a comment