Load unpacked extension: Select folder dialog shouldn't create non-existent folders |
|||||||||
Issue descriptionThe SelectFileDialog used to choose a directory to load from in chrome://extensions will create the directory if a non-existent directory is specified at the end of an otherwise valid path. It also includes a "Create Folder" option, which doesn't make sense in the context of selecting a folder with existing contents. 1. chrome://extensions 2. Ensure "Developer mode" is checked 3. "Load unpacked extension..." Expected: "Create Folder" button missing Actual: "Create Folder" button present and functional 4. In the Location bar of the dialog, type a non-existent folder name like "foo". Expected: Some kind of file-not-found error Actual: The directory is created on the file system. Then the Extensions page shows an error loading the extension's manifest file. Generally speaking, opening a directory to read from shouldn't write to the file system! Note: GTK provides an option to disable the "Create Folder" option, which I assume will also prevent the dialog from creating a folder that is manually typed into the dialog's location bar: https://developer.gnome.org/gtk3/stable/GtkFileChooser.html#gtk-file-chooser-set-create-folders To use this it would also need to be an option in the SelectFileDialog interface.
,
Oct 18 2017
michaelpg@ is this happening in the non-MD extensions? (Is it an MD extensions issue). I'd want to increase the priority if this is introduced by MD changes.
,
Oct 18 2017
No, this was the old/current chrome://extensions. It's a problem with our C++ file select dialog, not the extensions WebUI specifically, so it probably occurs in md-extensions as well.
,
Dec 11 2017
Issue 591540 has been merged into this issue.
,
Dec 11 2017
catmullings@, is this one you were interested in taking? (Pretty low priority, so no need to get to it right away)
,
Dec 13 2017
On Windows, you'd set |BIF_NONEWFOLDERBUTTON| as done in https://chromium-review.googlesource.com/#/c/chromium/src/+/777681
,
Feb 8 2018
,
Feb 8 2018
,
Mar 28 2018
,
Apr 6 2018
We currently have an enum type |ui::SelectFileDialog::SELECT_FOLDER| which is being used to select a folder for the download location as well as for loading unpacked extensions. In the select folder for the download location, it makes sense to allow creating a new folder from the folder picker. For selecting a location to load and unpacked extension a new enum can be added |ui::SelectFileDialog::SELECT_EXISTING_FOLDER| which would work like |ui::SelectFileDialog::SELECT_FOLDER| only that it would show a folder picker without a create folder option (where possible). In the KDE code, we're using kdialog. I haven't found an option to disable folder creation. For |ui::SelectFileDialog::SELECT_UPLOAD_FOLDER|, we still allow creating folders in gtk, kde and mac. Should that be updated?
,
Apr 6 2018
> For |ui::SelectFileDialog::SELECT_UPLOAD_FOLDER|, we still allow > creating folders in gtk, kde and mac. Should that be updated? Yes, as creating a new folder doesn't make sense in that scenario. I had fixed this on Windows in the CL in #6.
,
Apr 19 2018
,
Apr 19 2018
,
May 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cc859bf07d39ffb99f1e19827c894327fb4d9849 commit cc859bf07d39ffb99f1e19827c894327fb4d9849 Author: Esmael El-Moslimany <aee@chromium.org> Date: Thu May 24 00:59:47 2018 Extensions WebUI: add ui::SelectFileDialog::SELECT_EXISTING_FOLDER Use SELECT_EXISTING_FOLDER when choosing an unpacked extension. Disable folder creation for SELECT_EXISTING_FOLDER and SELECT_UPLOAD_FOLDER (for all desktop platforms excluding kde). For GTK when selecting a folder, show only folders. Bug: 772180 Change-Id: I0f513180ff7f10b5af91e8c518b3d9fd4b0fc8f1 Reviewed-on: https://chromium-review.googlesource.com/1019732 Commit-Queue: Esmael El-Moslimany <aee@chromium.org> Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#561344} [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/build/config/linux/gtk/BUILD.gn [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/build/install-build-deps.sh [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/chrome/browser/extensions/api/developer_private/developer_private_api.cc [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk.cc [add] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk_unittest.cc [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/chrome/browser/ui/libgtkui/select_file_dialog_impl_kde.cc [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/chrome/test/BUILD.gn [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/ui/shell_dialogs/select_file_dialog.h [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/ui/shell_dialogs/select_file_dialog_mac.mm [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/ui/shell_dialogs/select_file_dialog_mac_unittest.mm [modify] https://crrev.com/cc859bf07d39ffb99f1e19827c894327fb4d9849/ui/shell_dialogs/select_file_dialog_win.cc
,
May 24 2018
,
Jun 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eb43c62a34f5b7335a846d579143f8cc4cc777a8 commit eb43c62a34f5b7335a846d579143f8cc4cc777a8 Author: Yuri Wiitala <miu@chromium.org> Date: Thu Jun 07 20:45:00 2018 CrOS: Fix DCHECK failures in SelectFileDialog. Commit cc859bf07d39ffb99f1e19827c894327fb4d9849 introduced a new variant of select file dialog: SELECT_EXISTING_FOLDER. Because existing code used 'default' clauses in multiple switch statements, the compiler did not catch that there was missing impl for the ChromeOS UI. This change adds that missing impl. Bug: 772180 , 848303 Change-Id: Ic990bf838d3a8c300414259f246715ae2efdcfe3 Reviewed-on: https://chromium-review.googlesource.com/1089877 Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Yuri Wiitala <miu@chromium.org> Cr-Commit-Position: refs/heads/master@{#565396} [modify] https://crrev.com/eb43c62a34f5b7335a846d579143f8cc4cc777a8/chrome/browser/chromeos/file_manager/select_file_dialog_util.cc [modify] https://crrev.com/eb43c62a34f5b7335a846d579143f8cc4cc777a8/chrome/browser/chromeos/file_manager/url_util.cc [modify] https://crrev.com/eb43c62a34f5b7335a846d579143f8cc4cc777a8/chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk.cc [modify] https://crrev.com/eb43c62a34f5b7335a846d579143f8cc4cc777a8/chrome/browser/ui/libgtkui/select_file_dialog_impl_kde.cc
,
Jun 14 2018
For posterity/searching, the above change fixed the NOTREACHED() failure in GetDialogTypeAsString() because the dialog type (SELECT_EXISTING_FOLDER) wasn't handled. [164806:164806:0614/150828.184511:FATAL:url_util.cc(60)] Check failed: false. #0 0x7f5cd794c8fc base::debug::StackTrace::StackTrace() #1 0x7f5cd789c30b logging::LogMessage::~LogMessage() #2 0x562799e09410 file_manager::util::GetFileManagerMainPageUrlWithParams() #3 0x56279b4e0320 SelectFileDialogExtension::SelectFileImpl() #4 0x7f5cd20b40d3 ui::SelectFileDialog::SelectFile() #5 0x56279ac15669 extensions::api::EntryPicker::EntryPicker() #6 0x56279ac0d831 extensions::api::DeveloperPrivateLoadUnpackedFunction::Run() #7 0x5627997cf342 ExtensionFunction::RunWithValidation() #8 0x5627997d126d extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal() #9 0x5627997d0f0b extensions::ExtensionFunctionDispatcher::Dispatch() #10 0x5627997f59ca extensions::ExtensionWebContentsObserver::OnRequest() #11 0x5627997f5849 _ZN3IPC8MessageTI29ExtensionHostMsg_Request_MetaNSt3__15tupleIJ31ExtensionHostMsg_Request_ParamsEEEvE8DispatchIN10extensions28ExtensionWebContentsObserverES9_N7content15RenderFrameHostEMS9_FvPSB_RKS4_EEEbPKNS_7MessageEPT_PT0_PT1_T2_ #12 0x5627997f5792 extensions::ExtensionWebContentsObserver::OnMessageReceived() #13 0x56279ab5bb21 extensions::ChromeExtensionWebContentsObserver::OnMessageReceived() #14 0x7f5cd4f9f472 content::WebContentsImpl::OnMessageReceived() #15 0x7f5cd4c4b734 content::RenderFrameHostImpl::OnMessageReceived() #16 0x7f5cd4e6010b content::RenderProcessHostImpl::OnMessageReceived() #17 0x7f5cd6afeb41 IPC::ChannelProxy::Context::OnDispatchMessage() #18 0x7f5cd6b0166b _ZN4base8internal7InvokerINS0_9BindStateIMN3IPC12ChannelProxy7ContextEFvRKNS3_7MessageEEJ13scoped_refptrIS5_ES6_EEEFvvEE3RunEPNS0_13BindStateBaseE #19 0x7f5cd787f2c5 base::debug::TaskAnnotator::RunTask() #20 0x7f5cd78a8af9 base::internal::IncomingTaskQueue::RunTask() #21 0x7f5cd78ac15b base::MessageLoop::RunTask() #22 0x7f5cd78ac4ea base::MessageLoop::DeferOrRunPendingTask() #23 0x7f5cd78ac74c base::MessageLoop::DoWork() #24 0x7f5cd796c969 base::MessagePumpLibevent::Run() #25 0x7f5cd78abb44 base::MessageLoop::Run() #26 0x7f5cd78de449 base::RunLoop::Run() #27 0x56279a2011fb ChromeBrowserMainParts::MainMessageLoopRun() #28 0x7f5cd4ad6f87 content::BrowserMainLoop::RunMainMessageLoopParts() #29 0x7f5cd4ad9c26 content::BrowserMainRunnerImpl::Run() #30 0x7f5cd4ad3234 content::BrowserMain() #31 0x7f5cd54fcfa3 content::RunBrowserProcessMain() #32 0x7f5cd54fdf2d content::ContentMainRunnerImpl::Run() #33 0x7f5cd7bff8cc service_manager::Main() #34 0x7f5cd54fbfc4 content::ContentMain() #35 0x56279962f283 ChromeMain #36 0x7f5cc97f22b1 __libc_start_main #37 0x56279962f0fa _start Thanks!
,
Jun 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dac5bc2969e50cc116d11eccca55f2bb216b9af5 commit dac5bc2969e50cc116d11eccca55f2bb216b9af5 Author: Yuri Wiitala <miu@chromium.org> Date: Mon Jun 18 21:27:37 2018 CrOS: Fix DCHECK failures in SelectFileDialog. Commit cc859bf07d39ffb99f1e19827c894327fb4d9849 introduced a new variant of select file dialog: SELECT_EXISTING_FOLDER. Because existing code used 'default' clauses in multiple switch statements, the compiler did not catch that there was missing impl for the ChromeOS UI. This change adds that missing impl. Bug: 772180 , 848303 Change-Id: Ic990bf838d3a8c300414259f246715ae2efdcfe3 Reviewed-on: https://chromium-review.googlesource.com/1089877 Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Yuri Wiitala <miu@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#565396}(cherry picked from commit eb43c62a34f5b7335a846d579143f8cc4cc777a8) Reviewed-on: https://chromium-review.googlesource.com/1105237 Reviewed-by: Yuri Wiitala <miu@chromium.org> Cr-Commit-Position: refs/branch-heads/3440@{#421} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} [modify] https://crrev.com/dac5bc2969e50cc116d11eccca55f2bb216b9af5/chrome/browser/chromeos/file_manager/select_file_dialog_util.cc [modify] https://crrev.com/dac5bc2969e50cc116d11eccca55f2bb216b9af5/chrome/browser/chromeos/file_manager/url_util.cc [modify] https://crrev.com/dac5bc2969e50cc116d11eccca55f2bb216b9af5/chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk.cc [modify] https://crrev.com/dac5bc2969e50cc116d11eccca55f2bb216b9af5/chrome/browser/ui/libgtkui/select_file_dialog_impl_kde.cc |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by michae...@chromium.org
, Oct 6 2017