New issue
Advanced search Search tips

Issue 812171 link

Starred by 0 users

Issue metadata

Status: Verified
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

"Export passwords" button greyed out, but usable, after the user rotates their phone

Project Member Reported by vabr@chromium.org, Feb 14 2018

Issue description

Chrome 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.
 

Comment 1 by vabr@chromium.org, Feb 14 2018

Description: Show this description

Comment 2 by vabr@chromium.org, Feb 14 2018

Summary: "Export passwords" button greyed out, but usable, after the user rotates their phone (was: "Export passwords" button greyed out, but usable, after the user locks and unlocks their phone)
Sadly I cannot attach a screencast, because QuitTime fails to capture that as long as I start rotating the device.

Comment 3 by vabr@chromium.org, 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?

Comment 4 by ioanap@chromium.org, Feb 14 2018

Status: Started (was: Assigned)

Comment 5 by ioanap@chromium.org, 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.

Comment 6 by ioanap@chromium.org, Feb 16 2018

I know a fix for it now. CL will follow.
Project Member

Comment 7 by bugdroid1@chromium.org, 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

Comment 8 by ioanap@chromium.org, Feb 16 2018

Status: Fixed (was: Started)

Comment 9 by vabr@chromium.org, 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.
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.

Status: Verified (was: Fixed)
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
Cc: -vabr@chromium.org

Sign in to add a comment