Crostini apps should use mime type database from the container |
|||
Issue descriptionThe file manager uses the default mime type detection, which is codified in mime_util.cc (see kPrimaryMappings/kSecondaryMappings). We should consider also making use of the /etc/mime.types file inside the container, so that e.g. when chrome-side detection falls back to application/octet-stream, so we can pick more appropriate Crostini apps.
,
Aug 14
,
Aug 24
I've coded this up and will submit it soon; but I wanted to comment that the original examples aren't fixed by doing this unfortunately. The /etc/mime.types file in the container doesn't have an entry for .tga files and the mapping for .pcx files is image/pcx (whereas gimp is looking for image/x-pcx). However, with this change users would be able to edit/create a .mime.types file in their home directory and then have those values be used for associations (which could then be used to get .tga and .pcx files working properly).
,
Aug 29
"The file manager uses the default mime type detection," do you mean Files App? > The /etc/mime.types file in the container doesn't have an entry for .tga files and the mapping for .pcx files is image/pcx (whereas gimp is looking for image/x-pcx). Gaps. Still, why not use the "default mime detection" meaning network/mime_util.cc, I presume, to fill these inevitable mime gaps.
,
Aug 29
Yes, 'file manager' is 'Files app' in this context. The 'default mime detection' is already being used, and it is that which has all the gaps in it. That is what we are filling in with the /etc/mime.types in the container as a fallback detection technique. However certain types are in neither of them which is why gaps remain.
,
Aug 29
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/system_api/+/ca3489faea89ebf8c8db55b29a6e125028139ae6 commit ca3489faea89ebf8c8db55b29a6e125028139ae6 Author: Jeffrey Kardatzke <jkardatzke@google.com> Date: Wed Aug 29 23:09:29 2018 Add protobuf for mime type mappings in Crostini. BUG= chromium:869747 TEST=Builds Change-Id: Id2d59d432b8037a4fcc0f2d2c0c267fac5b088f2 Reviewed-on: https://chromium-review.googlesource.com/1188447 Commit-Ready: Jeffrey Kardatzke <jkardatzke@google.com> Tested-by: Jeffrey Kardatzke <jkardatzke@google.com> Reviewed-by: Dan Erat <derat@chromium.org> Reviewed-by: Stephen Barber <smbarber@chromium.org> [modify] https://crrev.com/ca3489faea89ebf8c8db55b29a6e125028139ae6/dbus/vm_applications/apps.proto [modify] https://crrev.com/ca3489faea89ebf8c8db55b29a6e125028139ae6/dbus/vm_applications/dbus-constants.h
,
Aug 30
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/9733c66c304069f5b55478c013879f4f071e2ada commit 9733c66c304069f5b55478c013879f4f071e2ada Author: Jeffrey Kardatzke <jkardatzke@google.com> Date: Thu Aug 30 04:05:57 2018 vm_tools: Add MIME type passing from container to host This parses the /etc/mime.types file in the container and passes that information along to the host so that the container's MIME types can be used for file extension mappings after we use the hardcoded ones in Chrome OS. Chrome OS has a list that is not very extensive, so this helps more file types get recognized properly by their corresponding app. It will also parse $HOME/.mime.types if present and use that as overrides to the system settings. BUG= chromium:869747 TEST=Unit tests pass, Open As shows proper options now CQ-DEPEND=CL:1188447 Change-Id: I6945cf8abaed97ec4f61fd22d16f02bc820d9f4a Reviewed-on: https://chromium-review.googlesource.com/1188793 Commit-Ready: Jeffrey Kardatzke <jkardatzke@google.com> Tested-by: Jeffrey Kardatzke <jkardatzke@google.com> Reviewed-by: Stephen Barber <smbarber@chromium.org> [modify] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/cicerone/container_listener_impl.cc [modify] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/proto/container_host.proto [modify] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/garcon/host_notifier.h [add] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/garcon/mime_types_parser.cc [add] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/garcon/mime_types_parser.h [modify] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/cicerone/service.cc [add] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/garcon/mime_types_parser_test.cc [modify] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/cicerone/service.h [modify] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/cicerone/container_listener_impl.h [modify] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/garcon/host_notifier.cc [modify] https://crrev.com/9733c66c304069f5b55478c013879f4f071e2ada/vm_tools/guest.gypi
,
Aug 30
> The 'default mime detection' is already being used, and it is that which has all the gaps in it. Main gap appears to be that the platform mine type part of mime_util.cc is implemented for all platforms _except Chrome_OS_.
,
Aug 30
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/6347998a88293221e39ef4517747fa86b308d33b commit 6347998a88293221e39ef4517747fa86b308d33b Author: Jeffrey Kardatzke <jkardatzke@google.com> Date: Thu Aug 30 08:28:15 2018 vm_guest_tools: Add mime_types unit test BUG= chromium:869747 TEST=Builds, unit tests pass CQ-DEPEND=CL:1188793 Change-Id: Ib03b396f12c989221b9edbeaa8a933d55d142988 Reviewed-on: https://chromium-review.googlesource.com/1188690 Commit-Ready: Jeffrey Kardatzke <jkardatzke@google.com> Tested-by: Jeffrey Kardatzke <jkardatzke@google.com> Reviewed-by: Stephen Barber <smbarber@chromium.org> [modify] https://crrev.com/6347998a88293221e39ef4517747fa86b308d33b/chromeos-base/vm_guest_tools/vm_guest_tools-9999.ebuild
,
Aug 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7618d52ad5b1814b7cac7d09a4545ee310190670 commit 7618d52ad5b1814b7cac7d09a4545ee310190670 Author: Jeffrey Kardatzke <jkardatzke@google.com> Date: Fri Aug 31 00:01:51 2018 Add alternate MIME types for files for Crostini This receives MIME type information from Crostini and uses that as a secondary lookup technique for file associations. This information is only referenced when a file has been detected to have one of the default MIME types of either text/plain or application/octet-stream. This is stored in a new preferences location as well with a unit test for that. Bug: 869747 Test: Verified Open With associations on eve, unit tests pass Change-Id: Ie631ae609b04c72e37c205e6d5c214816ba47563 Reviewed-on: https://chromium-review.googlesource.com/1195045 Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com> Reviewed-by: Dan Erat <derat@chromium.org> Reviewed-by: Timothy Loh <timloh@chromium.org> Reviewed-by: Nicholas Verne <nverne@chromium.org> Cr-Commit-Position: refs/heads/master@{#587863} [modify] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/BUILD.gn [add] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/crostini/crostini_mime_types_service.cc [add] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/crostini/crostini_mime_types_service.h [add] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/crostini/crostini_mime_types_service_factory.cc [add] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/crostini/crostini_mime_types_service_factory.h [add] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/crostini/crostini_mime_types_service_unittest.cc [modify] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/crostini/crostini_pref_names.cc [modify] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/crostini/crostini_pref_names.h [modify] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/crostini/crostini_remover.cc [modify] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/dbus/vm_applications_service_provider.cc [modify] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/dbus/vm_applications_service_provider.h [modify] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/browser/chromeos/file_manager/crostini_file_tasks.cc [modify] https://crrev.com/7618d52ad5b1814b7cac7d09a4545ee310190670/chrome/test/BUILD.gn
,
Aug 31
,
Sep 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/504cdddf5d1c97c02329770fb3d8854b982a1bbd commit 504cdddf5d1c97c02329770fb3d8854b982a1bbd Author: Jeffrey Kardatzke <jkardatzke@google.com> Date: Sat Sep 01 00:14:53 2018 Add test for alternate Crostini mime types Bug: chromium:869747 Test: Unit tests pass Change-Id: I40512bee8b797d2861e6492876e75bd36668fd58 Reviewed-on: https://chromium-review.googlesource.com/1199777 Reviewed-by: Joel Hockey <joelhockey@chromium.org> Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com> Cr-Commit-Position: refs/heads/master@{#588221} [modify] https://crrev.com/504cdddf5d1c97c02329770fb3d8854b982a1bbd/chrome/browser/chromeos/file_manager/file_tasks_unittest.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by dgreid@chromium.org
, Aug 6Status: Assigned (was: Untriaged)