Pull in file chooser from Chromium #349
Conversation
|
we can remove |
patches/master_patch.patch
Outdated
| - InfoBarService::FromWebContents(source_contents_), | ||
| - infobars::InfoBarDelegate::CHROME_SELECT_FILE_POLICY, nullptr, | ||
| - l10n_util::GetStringUTF16(IDS_FILE_SELECTION_DIALOG_INFOBAR), true); | ||
| +// SimpleAlertInfoBarDelegate::Create( |
There was a problem hiding this comment.
We can use #if defined(MUON_CHROMIUM_BUILD) to comment this block
patches/master_patch.patch
Outdated
| --- a/chrome/browser/ui/chrome_select_file_policy.cc | ||
| +++ b/chrome/browser/ui/chrome_select_file_policy.cc | ||
| @@ -30,10 +30,12 @@ bool ChromeSelectFilePolicy::CanOpenSelectFileDialog() { | ||
| void ChromeSelectFilePolicy::SelectFileDenied() { | ||
| // Show the InfoBar saying that file-selection dialogs are disabled. | ||
| if (source_contents_) { | ||
| +#if !defined(MUON_CHROMIUM_BUILD) | ||
| +#ifdef MUON_CHROMIUM_BUILD |
There was a problem hiding this comment.
This might be confusing.
MUON_CHROMIUM_BUILD is actually Build chromium without muon
see
muon/patches/master_patch.patch
Line 37 in a0d913d
so the previous change is the right one
| @@ -637,6 +637,14 @@ BraveBrowserContext::GetIOTaskRunner() { | |||
| GetPath(), BrowserThread::GetBlockingPool()); | |||
| } | |||
|
|
|||
| base::FilePath BraveBrowserContext::last_selected_directory() { | |||
| return GetPrefs()->GetFilePath(prefs::kSelectFileLastDirectory); | |||
There was a problem hiding this comment.
we actually have two prefs files. One of them is leftover from electron and is a PrefRegistrySimple so it wasn't compatible with other changes, but it still has a few prefs in it and kSelectFileLastDirectory is one of them AtomBrowserContext::RegisterPrefs. Checking with @bbondy to see if we care about migrating it (in which case we should handle it like kDownloadDefaultDirectory), otherwise we should remove it from AtomBrowserContext
There was a problem hiding this comment.
also it should be registered in BraveBrowserContext with RegisterFilePathPref
There was a problem hiding this comment.
sounds like it's fine to just remove the old pref from AtomBrowserContext
fix brave/browser-laptop#10666
fix brave/browser-laptop#8601
Auditors: @darkdh, @bbondy, @bsclifton, @bridiver