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

Issue 857088 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jul 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug
Q2



Sign in to add a comment

Previously selected item deselects on using history search

Project Member Reported by rakurati@chromium.org, Jun 27 2018

Issue description

App Version: 69.0.3473.0 Canary
iOS Version: 10.3.3, 11.4.1 beta 3, 11.4
Device: iPhone and iPad 

Steps to reproduce:
1. Fresh install chrome and Launch chrome
2. Browse through few sites
3. Open History and tap on edit button
4. Search for the website which is already browsed in history search
5. Select the item from the results
6. Repeat step 4 and 5 again
7. Dismiss the search by tapping on cancel button 

Observed results:
Notice that initially selected history item deselected on selecting new history item

Expected results:
Using history search should allow to select multiple history items

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes/No
Bug reproducible after clearing cache and cookies: Yes/No
Bug reproducible on Chrome Mobile on Android: Not tested
Bug reproducible on Safari/Firefox: Firefox: NA, Safari: NA
Bug reproducible on current stable build (App Version, iOS Version): NA on M67 (Search button disabled in edit mode )
Bug reproducible on the current beta channel build (App Version, iOS Version): NA on M68 (Search button disabled in edit mode )

Link to video/image:
https://drive.google.com/file/d/1ly7M_PrS8Hvc7M4eO5-LAZMnzJKYMGh_/view?usp=sharing

 

Comment 1 by sczs@chromium.org, Jun 27 2018

Owner: sczs@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by marq@chromium.org, Jun 28 2018

Labels: -Pri-2 Q2 Pri-1
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 28 2018

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

commit ceca5e061d00c65050647b5c6ad6c843c7e0ca7c
Author: sczs <sczs@chromium.org>
Date: Thu Jun 28 23:45:53 2018

[ios] Improves History UIRefresh search filtering.

Previously history filtering was using selectRowAtIndexPath: to select the rows that were going to
be filtered by the searchController. This caused that previously selected cells on edit mode were
filtered out when searching. This CL introduces an Array to hold these indexes to filter out later.

Also, when there're no results when search filtering, the emptyTableView shouldn't be displayed.

Bug:  857088 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I40bd4e5181d84fc23edae1eeea64e899aa120d4f
Reviewed-on: https://chromium-review.googlesource.com/1118909
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571326}
[modify] https://crrev.com/ceca5e061d00c65050647b5c6ad6c843c7e0ca7c/ios/chrome/browser/ui/history/history_table_view_controller.mm

Comment 4 by sczs@chromium.org, Jun 28 2018

Status: Fixed (was: Assigned)
Status: Assigned (was: Fixed)
Verified on iPhone X iOS 11.4 69.0.3480.0 Canary.
This bug is still reproducible.
Thanks.
Cc: ghendel@chromium.org mard...@chromium.org martijnb@chromium.org
Labels: -Pri-1 Pri-3
Thanks vbhatsoori@!

If on step 6 we search and completely remove the first selected item, then this item will be deselected.
e.g.
Search for youtube.com and select that item.
Then search for google.com (this will completely remove youtube.com and unselect it).

This is the behavior on LegacyHistory as well, and its how UIRefresh was designed.
If we want to persist selection through different searches we need to treat this as a feature request.

The original bug was that if we search for "youtu", select the youtube.com item, and then add a "b" and make it "youtub" all selected items would be deselected even if they are still on the screen. That's what the previous patch fixed.

+mardini,gabe,martijn, WDYT about adding this as a feature later on? In any case I'm lowering this priority.
I would be fine with adding this as a feature later. Don't think it's very common user behaviour. 
I agree with Martijn. 
Status: Fixed (was: Assigned)
Thanks, sg!

vbhatsoori@ or rakurati@ could you please file a new bug/feature with the part that wasn't fix and assign it to me? Thanks!
Status: Verified (was: Fixed)
Verified in 69.0.3482.0 Canary in iPad (iOS 11.4), iPhone X(iOS 11.4.1) 

This issue is now fixed as mentioned in comment#6

Link to video: 
https://drive.google.com/file/d/1EDhCXkxKe1R7E3tnZjsb1mFdrhSnGk4A/view?usp=sharing

@sczs, new feature request ticket is raised against the part that wasn't fix http://crbug/860407

Sign in to add a comment