New issue
Advanced search Search tips

Issue 908122 link

Starred by 9 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

crostini - x-www-browser can't open file urls

Reported by jfgio...@gmail.com, Nov 23

Issue description

UserAgent: Mozilla/5.0 (X11; CrOS x86_64 11265.0.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3611.0 Safari/537.36
Platform: 11265.0.0 (Official Build) dev-channel eve

Steps to reproduce the problem:
1. open a crostini terminal
2. type x-www-browser file:///media/ (or any valid file url)

What is the expected behavior?
Chrome should open a new tab with the url.

What went wrong?
we get an error:
[1123/165229:WARNING:host_notifier.cc(134)] Failed to request host system to open url "file:///media/" error: Failure in OpenUrl

Did this work before? N/A 

Chrome version: 72.0.3611.0  Channel: dev
OS Version: 11265.0.0
Flash Version: 

the same url works fine when typed direct in a Crome tab.
same issue when using "$BROWSER url"
 
Components: OS>Systems>Containers
Owner: nverne@chromium.org
Status: WontFix (was: Unconfirmed)
Traced this to cicerone - can't call the dbus  org.chromium.UrlHandlerServiceInterface.OpenUrl. Failed with Invalid URL

This is because file: url schemes aren't supported.
https://cs.chromium.org/chromium/src/ash/dbus/url_handler_service_provider.cc?type=cs&g=0&l=21
shows we only support http:, https:, ftp: and mailto:

I believe this is by design, so marking as WAI. 
we prob should look into whether we can support it reasonably.  we would have to handle translating the path so we go through the Files app mount.  file:// usage isn't uncommon (or that unreasonable).
I agree some version of this would be useful to support. Consider a developer writing Markdown within penguin and wanting to preview it. Typical Markdown previewers like `moo` pass a `file://` URL to the browser.

If the file path is visible outside the container and Chrome could otherwise access the path, a path translation could be done.
Viewing local pdf files is a frequent use case. 
Owner: joelhockey@chromium.org
Status: Started (was: WontFix)
I have some CLs to allow file:// urls and do the path translations.
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 9

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

commit dd328fe42a82d88d6950948254d89abd9fcaf9b4
Author: Joel Hockey <joelhockey@chromium.org>
Date: Wed Jan 09 22:04:16 2019

Allow file scheme urls for org.chromium.UrlHandlerService

A follow up CL will modify cicerone to do path translations.
$HOME => file://media/fuse/crostini_<cryptohome-hash>_termina_penguin
/mnt/chromeos/MyFiles => /home/chronos/u-<cryptohome-hash>/MyFiles
/mnt/chromeos/GoogleDrive => /media/fuse/drivefs-<drivefs-hash>
/mnt/chromeos/PlayFiles => /run/arc/sdcard/write/emulated/0
other: Not visible and cicerone will fail before sending.

Bug: 908122
Change-Id: I566021713541edf4a849cf93ced8c315bb4a87c4
Reviewed-on: https://chromium-review.googlesource.com/c/1401924
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621304}
[modify] https://crrev.com/dd328fe42a82d88d6950948254d89abd9fcaf9b4/ash/dbus/url_handler_service_provider.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/55ea1bfa5ae3bd638399d075f9c38f29856bc87e

commit 55ea1bfa5ae3bd638399d075f9c38f29856bc87e
Author: Joel Hockey <joelhockey@chromium.org>
Date: Thu Jan 10 12:49:53 2019

vm_tools: garcon: Convert OpenUrl local file to file://<abs_path>

BUG=chromium:908122
TEST='x-www-browser pdf-test.pdf' in container

Change-Id: Ie45a81f7fbd25c724fb13235ea27e88a7451a77f
Reviewed-on: https://chromium-review.googlesource.com/1401969
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/55ea1bfa5ae3bd638399d075f9c38f29856bc87e/vm_tools/garcon/host_notifier.cc

Cc: jkardatzke@chromium.org vapier@chromium.org joelhockey@chromium.org jorgelo@chromium.org dominickn@chromium.org tbuck...@chromium.org mutexlox@chromium.org
 Issue 899930  has been merged into this issue.
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4d4b572d4397c7070d5092cb7a0608752140f661

commit 4d4b572d4397c7070d5092cb7a0608752140f661
Author: Joel Hockey <joelhockey@chromium.org>
Date: Wed Jan 16 09:46:48 2019

system_api: Add drivefs_mount_path to cicerone StartLxdContainer

BUG=chromium:908122
TEST='x-www-browser /mnt/chromeos/GoogleDrive/MyDrive/pdf-test.pdf' in container

Change-Id: Ibf138ae95807109400fa215243711c4461f628ae
Reviewed-on: https://chromium-review.googlesource.com/1406109
Commit-Ready: Joel Hockey <joelhockey@chromium.org>
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/4d4b572d4397c7070d5092cb7a0608752140f661/system_api/dbus/vm_cicerone/cicerone_service.proto

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/9a112b46b74b75627edf0a3687f1417c4f5d5e3e

commit 9a112b46b74b75627edf0a3687f1417c4f5d5e3e
Author: Joel Hockey <joelhockey@chromium.org>
Date: Wed Jan 16 09:46:49 2019

vm_tools: cicerone: Store drivefs_mount_path from LxdStartContainer

BUG=chromium:908122
TEST='x-www-browser /mnt/chromeos/GoogleDrive/MyDrive/pdf-test.pdf' in container

Change-Id: Idf780bc5414521197bb12fb9923b0d769f6f839a
Reviewed-on: https://chromium-review.googlesource.com/1406110
Commit-Ready: Joel Hockey <joelhockey@chromium.org>
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/9a112b46b74b75627edf0a3687f1417c4f5d5e3e/vm_tools/cicerone/service.cc
[modify] https://crrev.com/9a112b46b74b75627edf0a3687f1417c4f5d5e3e/vm_tools/cicerone/container.cc
[modify] https://crrev.com/9a112b46b74b75627edf0a3687f1417c4f5d5e3e/vm_tools/cicerone/container.h

Comment 12 by joelhockey@chromium.org, Jan 16 (6 days ago)

FYI: original URL open code added in: https://chromium-review.googlesource.com/c/chromium/src/+/998081/
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 17 (6 days ago)

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

commit 55411dc5c0adb1c31e04b7803775f6e6a141f54a
Author: Joel Hockey <joelhockey@chromium.org>
Date: Thu Jan 17 03:15:16 2019

Send drivefs_mount_path in StartLxdContainer

Bug: 908122
Change-Id: I73e1be440e87731210494acb180a6773a6d2794e
Reviewed-on: https://chromium-review.googlesource.com/c/1405950
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623546}
[modify] https://crrev.com/55411dc5c0adb1c31e04b7803775f6e6a141f54a/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/55411dc5c0adb1c31e04b7803775f6e6a141f54a/chrome/browser/chromeos/crostini/crostini_manager_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 19 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2a26a7641b4acc76481757e619f93dd705500699

commit 2a26a7641b4acc76481757e619f93dd705500699
Author: Joel Hockey <joelhockey@chromium.org>
Date: Sat Jan 19 04:05:14 2019

vm_tools: cicerone: Translate OpenUrl paths for host

BUG=chromium:908122
TEST='x-www-browser pdf-test.pdf' in container

Change-Id: Ic1d61ca936d1a3490e33b2e25684a067602e7938
Reviewed-on: https://chromium-review.googlesource.com/1401968
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/2a26a7641b4acc76481757e619f93dd705500699/vm_tools/cicerone/service.cc
[modify] https://crrev.com/2a26a7641b4acc76481757e619f93dd705500699/vm_tools/cicerone/container.cc
[modify] https://crrev.com/2a26a7641b4acc76481757e619f93dd705500699/vm_tools/cicerone/container.h

Sign in to add a comment