"Export passwords" button greyed out, but usable, after the user rotates their phone |
||||||
Issue descriptionChrome Version : 66.0.3343.0 OS Version: iOS device: iPhone 5S What steps will reproduce the problem? 0. Ensure there is a good number of passwords saved (couple of tens?) to require scrolling on the passwords settings page to reach its end. 1. In portrait mode, open Settings > Save Passwords > scroll down, tap Export Passwords..., then confirm export, authenticate, wait for the offer to share with some apps 2. Rotate the device to landscape mode. Notice that the "Export Passwords..." button is pushed below the screen border and becomes invisible. 3. Cancel the sharing prompt and scroll down to the Export button. What is the expected result? The "Export Passwords..." button is enabled What happens instead of that? The "Export Passwords..." button is greyed-out, but tapping it does trigger the export flow again. Assigning to ioanap@ for now, because I'm not sure I will get to investigating this until Tuesday.
,
Feb 14 2018
Sadly I cannot attach a screencast, because QuitTime fails to capture that as long as I start rotating the device.
,
Feb 14 2018
The fact that the greyed-out button works means that exportEnabled_ is true in save_passwords_collection_view_controller.mm. That means that updateExportPasswordsButton has requested colouring the button with [[MDCPalette greyPalette] tint900], which should be the "enabled" colour. So perhaps this is indeed some problem lower down in the framework?
,
Feb 14 2018
,
Feb 15 2018
Thank you for looking into this! I found out that the reason the color doesn't get updated is that, while the element is not visible, the cell returned is nil, so - reconfigureCellsForItems: can't update it. I am not sure what the right way to update the button is in this case. I will look into it.
,
Feb 16 2018
I know a fix for it now. CL will follow.
,
Feb 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a29b20ea9f2b1e02b76412f233321348889b7c0 commit 0a29b20ea9f2b1e02b76412f233321348889b7c0 Author: Ioana Pandele <ioanap@chromium.org> Date: Fri Feb 16 22:48:24 2018 Disable prefetching on SavePasswords When prefetching is enabled, the collection view can get configured cells in advance of them becoming visible. If the item updates after a cell was prefetched, but before it is visible, the cell cannot be reconfigured. For SavePasswords, the Export button may need to be enabled/disabled after being prefetched, so prefetching needs to be disabled. Bug: 812171 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: Ieef7b9cc3b91e237638277654e5d373c3906fb47 Reviewed-on: https://chromium-review.googlesource.com/924307 Reviewed-by: edchin <edchin@chromium.org> Commit-Queue: Ioana Pandele <ioanap@chromium.org> Cr-Commit-Position: refs/heads/master@{#537441} [modify] https://crrev.com/0a29b20ea9f2b1e02b76412f233321348889b7c0/ios/chrome/browser/ui/settings/save_passwords_collection_view_controller.mm
,
Feb 16 2018
,
Feb 19 2018
Thanks for the amazingly fast fix, Ioana, and for the detective work! Do you know whether disabling the prefetching will have any adversary effect on how lists with many passwords are handled? The Bling team (and Pink in particular) brought up the concern of how fast Chrome handles hundreds of passwords. Seeing that exporting is a rarely used feature and this particular bug a low-impact one, we might need to reconsider fixing it if it made life harder for the users on more frequent occasions.
,
Feb 19 2018
Prefetching loads a few cells in advance, which in some cases can improve scrolling speed. I might be wrong, but in the passwords' case, populating a cell should not be very time consuming, since the text we need is readily available in the list of passwords that is loaded when entering the Save Passwords view.
,
Apr 11 2018
Issue verified Version: Chrome Beta 66.0.3359.104 Device: iPhone 5S iOS: 11.2.6 The "Export Passwords..." button is enabled https://drive.google.com/open?id=1T57CZST5rGOnOZ0KJoR9SSSDlVbfGJrc
,
Nov 29
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by vabr@chromium.org
, Feb 14 2018