New issue
Advanced search Search tips

Issue 800880 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Task

Blocking:
issue 778276



Sign in to add a comment

MediaViewerActivity: Determine file URI when possible

Project Member Reported by steimel@chromium.org, Jan 10 2018

Issue description

Currently in MediaViewerActivity, we send the content:// URI off as the URI to display to the user. However, we want to show the user the file:// URI instead when possible.
 
Blocking: 778276
Labels: -Pri-3 M-69 Pri-1
Could we show the file name instead of the URI? It sounds that it would be more beneficial for the user than a very long string they can't parse.
Labels: -Pri-1 Pri-2
Cc: emilyschechter@chromium.org
+emilyschechter any concerns from a security UX perspective about this (or should we ask security eng instead)?  In my opinion showing the file:// path will be much more useful than showing the content:// URI in terms of understanding what is being shown?

I guess we need to be sure that we're showing the file:// scheme always in the case to avoid someone doing something like:  file://totalysanelookingfile.www.google.com and having the trailing bit being shown in the omnibox.

@steimel, can you upload a couple screenshots of the pre-post UI to make it a bit clearer?
Per our offline discussion, we're actually not currently planning on showing the file:// URI for M69. Instead, we're just going to show the file display name as the title. Attached is a screencast where I open a video and it shows the video name as the title.
media_intent_handler_demo_updated.mp4
5.8 MB View Download
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 13

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

commit 22f1e08c7751380cfb981cac68fc06652581444d
Author: Tommy Steimel <steimel@chromium.org>
Date: Mon Aug 13 19:33:24 2018

[Media Intent Handler] Use file display name as title for content URIs

This CL changes NavigationEntryImpl::GetTitleForDisplay for content
URIs to use the file's display name if determinable. This affects HTML,
audio, and video files opened with Chrome on Android. This does not
affect image files since ImageDocument already has a built-in title.

Bug:  800880 
Change-Id: I0ad03d2dc13d4895f6789a6a7a6ea1199e19745b
Reviewed-on: https://chromium-review.googlesource.com/1165790
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582664}
[modify] https://crrev.com/22f1e08c7751380cfb981cac68fc06652581444d/base/android/content_uri_utils.cc
[modify] https://crrev.com/22f1e08c7751380cfb981cac68fc06652581444d/base/android/content_uri_utils.h
[modify] https://crrev.com/22f1e08c7751380cfb981cac68fc06652581444d/base/android/java/src/org/chromium/base/ContentUriUtils.java
[modify] https://crrev.com/22f1e08c7751380cfb981cac68fc06652581444d/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/22f1e08c7751380cfb981cac68fc06652581444d/content/browser/frame_host/navigation_entry_impl_unittest.cc

Labels: Merge-Request-69
Status: Fixed (was: Assigned)
Merge request for the CL in comment 7. This solves a UX blocker for the media intent handler launch (which is approved for M69). I waited on it since there was one issue with it which is now fixed in crbug/874149 (which I will also merge request). Thanks!
Project Member

Comment 9 by sheriffbot@chromium.org, Aug 17

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-69 Merge-Approved-69
Approved for merge to 69, branch 3497.
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 20

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/207c72adfe3aa9621f0950e72ccef8e21d242fa2

commit 207c72adfe3aa9621f0950e72ccef8e21d242fa2
Author: Tommy Steimel <steimel@chromium.org>
Date: Mon Aug 20 16:51:30 2018

[Media Intent Handler] Use file display name as title for content URIs

This CL changes NavigationEntryImpl::GetTitleForDisplay for content
URIs to use the file's display name if determinable. This affects HTML,
audio, and video files opened with Chrome on Android. This does not
affect image files since ImageDocument already has a built-in title.

Bug:  800880 
Change-Id: I0ad03d2dc13d4895f6789a6a7a6ea1199e19745b
Reviewed-on: https://chromium-review.googlesource.com/1165790
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#582664}(cherry picked from commit 22f1e08c7751380cfb981cac68fc06652581444d)
Reviewed-on: https://chromium-review.googlesource.com/1181581
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#713}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/207c72adfe3aa9621f0950e72ccef8e21d242fa2/base/android/content_uri_utils.cc
[modify] https://crrev.com/207c72adfe3aa9621f0950e72ccef8e21d242fa2/base/android/content_uri_utils.h
[modify] https://crrev.com/207c72adfe3aa9621f0950e72ccef8e21d242fa2/base/android/java/src/org/chromium/base/ContentUriUtils.java
[modify] https://crrev.com/207c72adfe3aa9621f0950e72ccef8e21d242fa2/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/207c72adfe3aa9621f0950e72ccef8e21d242fa2/content/browser/frame_host/navigation_entry_impl_unittest.cc

Sign in to add a comment