Bookmarks search results doesn’t return folders |
||||
Issue descriptionApp Version: 71.0.3555.0 Canary iOS Version: 11.4.1, 10.3.3, 12 GM Device: iPhone, iPad Pre condition: Bookmark should contain few websites and folders Steps to reproduce: 1. Launch chrome 2. Go to bookmarks 3. Search for a folder name Observed results: Notice that folder name is not shown in results Expected results: Folder name should be returned in results 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: Not tested Bug reproducible on Safari/Firefox: Firefox: No, Safari: No Bug reproducible on current stable build (App Version, iOS Version): No on M69 (Feature available from M71) Bug reproducible on the current beta channel build (App Version, iOS Version): No on M70 beta (Feature available from M71) Link to video: https://drive.google.com/file/d/1PjL6at2M9cjdHrwgrRLYzKmrAPinh25P/view?usp=sharing
,
Sep 20
djean@ as he implemented this feature. I think this is by design, and not sure if the initial idea was to display folders or not.
,
Sep 21
I am actually filtering out folders, but I don't remember why. I'll have another look.
,
Sep 28
Ok, now I remember why. Because we stack folder controllers during navigation, if we allow going to any folder from a search anywhere in the folder tree, then we would have to pop N folder controllers then push down M folder controllers. I don't see an easier way. We would of course have to animate only the last push or pop.
,
Sep 28
Ok, calling [self.navigationController setViewControllers:stack animated:YES] with a new stack works quite well. I tried to optimize a series of pop/push with IRT a common ancestor, but that didn't work well. So I'll keep the full stack replacement.
,
Sep 28
So once we select a folder we'll exit search mode and move to the folder, right? That'd be pretty cool, hopefully the full stack replacement animation looks well!
,
Oct 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9bc8b198e07a27c860749ff4a40e37257e10c723 commit 9bc8b198e07a27c860749ff4a40e37257e10c723 Author: David Jean <djean@google.com> Date: Wed Oct 03 07:33:53 2018 [ios] Allow searching for bookmark folders - Stop filtering folders out of search results in mediator. - When user selects a Folder in search results, recompute and reset the stack of view controllers, so that the user is in proper place in the bookmark panel stacks for back button functionality. Video: https://drive.google.com/file/d/18-dhC7ZE4j5tounA9I6LiLNGBtTVyTJM/view?usp=sharing Bug: 886612 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: I7ac42afcaba937b278548f3a37b8d07b87dc4a67 Reviewed-on: https://chromium-review.googlesource.com/c/1254142 Commit-Queue: David Jean <djean@chromium.org> Reviewed-by: Sergio Collazos <sczs@chromium.org> Reviewed-by: Gauthier Ambard <gambard@chromium.org> Cr-Commit-Position: refs/heads/master@{#596141} [modify] https://crrev.com/9bc8b198e07a27c860749ff4a40e37257e10c723/ios/chrome/browser/ui/bookmarks/bookmark_home_mediator.mm [modify] https://crrev.com/9bc8b198e07a27c860749ff4a40e37257e10c723/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm [modify] https://crrev.com/9bc8b198e07a27c860749ff4a40e37257e10c723/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm
,
Oct 5
,
Oct 9
Verified on 71.0.3572.0 Canary, iPhone X iOS 11.4.1 Looks good. |
||||
►
Sign in to add a comment |
||||
Comment 1 by kkhorimoto@chromium.org
, Sep 20Owner: sczs@chromium.org
Status: Assigned (was: Untriaged)