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

Issue 811317 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
hobby only
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 788701



Sign in to add a comment

Password Export: rename default file name

Project Member Reported by maxwalker@chromium.org, Feb 12 2018

Issue description

Chrome Version: 66.0.3344.0
OS: Android

What steps will reproduce the problem?
(1) Export passwords
(2) Save to Drive via the share sheet

Could you change the default file name to “Chrome Passwords”, please?
Current name is "pwd-export[+20 digit number]".

Thanks!
 
Save To Drive.png
630 KB View Download

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

Blocking: 788701

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

Just some background on this:

The displayed file name is the name of a temporary file which is referenced from the sharing intent. Ideally, I would be able to supply a file name suggestion to the consuming app and hide the temporary filename, which is just an implementation detail.

However, so far I could not find a way to do that. A workaround would be to use the user-friendly name also for the temporary file. During implementing the feature, a reviewer pointed out [2] that this might not be the best idea. The biggest disadvantages there seem to be:
(1) Exotic characters in the filename (note that "Chrome Passwords" is translated, and it should be, if it is meant for humans).
(2) Two subsequent exports reusing the same file.

As for (2), specifically, the potential issues are:
 * If the user exports, then lets, e.g., Drive upload the file, then goes quickly back and exports again for another purpose, the file being uploaded can be overwritten, resulting in a corrupted upload.
 * If, for some reason, the first export crashes and leaves the cached file's access rights in an inconsistent state, the subsequent exports won't work until the cache is cleared.

I fear that the risks are heavier than the inconvenience caused by the cryptic name. So I'd propose to look further for a systematic solution, and if there is not any, keeping this as a known issue.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/793838/9/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java#292
Cc: cfroussios@chromium.org
Could we append a (human readable, localized) timestamp to resolve the issue of overwritten files? For example: "Chrome Passwords (January 14 2018, 14:05:15)"?

Adding cfroussios@ who has been thinking about this on the desktop side.

Comment 4 by vabr@chromium.org, Feb 13 2018

So apparently, when I set the Intent.EXTRA_SUBJECT field of the share intent, then Drive will use that as a filename. This allows me to let Drive show a human friendly name while keeping the temp file name unique.

This does not apply just to Drive, though. Other apps can use that too. For example, a mailing app (such as GMail) would use the field content as the subject of the e-mail. That might or might not be desired, but in general, saying that the "subject" of the sharing intent is "Chrome Passwords" makes sense to me.

@maxwalker, do you want me to proceed with making the human-friendly name the "subject" of the sharing intent? Or do you want me to try to make the human-friendly name also unique, as outlined in #3?
Thanks for looking into it! Yes, using the human-friendly name the subject of the sharing intent sounds reasonable to me, too.

Comment 6 by vabr@chromium.org, Feb 13 2018

Status: Started (was: Assigned)
Thanks for the quick turn-around, Max! I'll make a patch to implement the "subject" solution.

Comment 7 by vabr@chromium.org, Feb 13 2018

Cl at https://crrev.com/c/916241
File selection on desktop typically warns when you're about to overwrite a file. Appending the date to the name shouldn't be a problem, but it's also not really needed.

I didn't understand if it's still desirable to introduce this on desktop. If you want it, let me know
Thank you, Vaclav!

No need for changes on desktop. I was just wondering whether there are parts of the desktop implementation that could be helpful for Android. :)
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 13 2018

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

commit d93d983001ddc4218981d4882e4af7410e263390
Author: Vaclav Brozek <vabr@chromium.org>
Date: Tue Feb 13 20:47:16 2018

[Android] Set EXTRA_SUBJECT for sharing exported passwords

When exporting passwords, Chrome creates a share intent with an
associated temporary file containing the passwords. This CL ensures
that the intent contains a human-readable EXTRA_SUBJECT field. This
allows, e.g., Google Drive app to pre-fill the subject as the new
filename, instead of reusing the cryptic-looking temporary file name.

Bug:  811317 
Change-Id: Ic3a3a0276c4ad5a98f05c191ee585f00a6551690
Reviewed-on: https://chromium-review.googlesource.com/916241
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536461}
[modify] https://crrev.com/d93d983001ddc4218981d4882e4af7410e263390/chrome/android/java/src/org/chromium/chrome/browser/preferences/password/SavePasswordsPreferences.java
[modify] https://crrev.com/d93d983001ddc4218981d4882e4af7410e263390/chrome/android/java/strings/android_chrome_strings.grd

Comment 11 by vabr@chromium.org, Feb 13 2018

Status: Fixed (was: Started)

Sign in to add a comment