-
Notifications
You must be signed in to change notification settings - Fork 893
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Allow downloading PDFs instead of opening them in Brave.
Fixes brave/brave-browser#1531 The fix does the following: 1. Makes the initial decision on whether to load PDFJS extension based on the value of kPluginsAlwaysOpenPdfExternally profile preference in addition to the command line switch. 2. Watches profile preference value kPluginsAlwaysOpenPdfExternally and adds/removes PDFJS component when the value changes. 3. Modifies js behind the chrome://settings/content/pdfDocuments so that if the PDFJS extension is disabled from the command line the option to download PDF files instead of opening them in Brave is set to ON and the toggle is disabled. 4. Adds an alternative call for whitelisted extensions in extensions/common/manifest_handlers/mime_types_handler.h that removes Chrome's PDF Extension ID from whitelisted IDs and patches chrome/browser/plugins/plugin_utils.cc to use the alternative call. This is done because on Linux that extension is not removed from profile enabled extensions (because there is no extensions install verification) and it ends up being picked as a handler for PDFs. If our PDFJS extension is disabled via command line, this causes a PDF to be neither displayed nor downloaded. Note, that this fix doesn't address being able to turn off opening PDFs in Brave in Guest/Tor profiles. The web ui setting to do so is not available in these profiles. The command line switch to not load PDFJS already applies to all profile types. Adds browser tests to check that Chromium's PDF extension is not considered for handling PDFs and if the PDFJS extension is not loaded or disabled from command line a download is initiated when navigating to a PDF URL. Also adds test to verify that when PDF download preference is toggled the component loader will take an appropriate action (add or remove PDFJS extension).
- Loading branch information
Showing
10 changed files
with
434 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| /* This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this file, | ||
| * You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| #include "brave/browser/extensions/brave_component_loader.h" | ||
| #include "brave/browser/extensions/brave_extension_functional_test.h" | ||
| #include "brave/common/brave_switches.h" | ||
| #include "chrome/browser/extensions/extension_service.h" | ||
| #include "chrome/browser/profiles/profile.h" | ||
| #include "chrome/browser/ui/browser.h" | ||
| #include "chrome/common/pref_names.h" | ||
| #include "components/prefs/pref_service.h" | ||
| #include "content/public/test/browser_test_utils.h" | ||
| #include "testing/gtest/include/gtest/gtest.h" | ||
|
|
||
| using extensions::BraveComponentLoader; | ||
|
|
||
| class BravePDFExtensionTest : public extensions::ExtensionFunctionalTest, | ||
| public BraveComponentLoader::TestingCallbacks { | ||
| public: | ||
| BravePDFExtensionTest() : pdf_extension_action_(TestingCallbacks::NONE) {} | ||
| ~BravePDFExtensionTest() override = default; | ||
|
|
||
| protected: | ||
| void SetUpOnMainThread() override { | ||
| extensions::ExtensionService* service = | ||
| extensions::ExtensionSystem::Get(profile())->extension_service(); | ||
| DCHECK(service); | ||
| (static_cast<BraveComponentLoader*>(service->component_loader())) | ||
| ->set_testing_callbacks(this); | ||
| } | ||
|
|
||
| // BraveComponentLoader::TestingCallbacks | ||
| void OnPdfExtensionAction( | ||
| TestingCallbacks::PdfExtensionAction action) override { | ||
| pdf_extension_action_ = action; | ||
| } | ||
|
|
||
| void SetDownloadPDFs(bool value) { | ||
| DCHECK(browser()); | ||
| profile()->GetPrefs()->SetBoolean(prefs::kPluginsAlwaysOpenPdfExternally, | ||
| value); | ||
| } | ||
|
|
||
| TestingCallbacks::PdfExtensionAction pdf_extension_action() { | ||
| return pdf_extension_action_; | ||
| } | ||
|
|
||
| private: | ||
| TestingCallbacks::PdfExtensionAction pdf_extension_action_; | ||
| }; | ||
|
|
||
| IN_PROC_BROWSER_TEST_F(BravePDFExtensionTest, ToggleDownloadPDFs) { | ||
| // Set preference to always download PDFs. | ||
| SetDownloadPDFs(true); | ||
| EXPECT_EQ(TestingCallbacks::WILL_REMOVE, pdf_extension_action()); | ||
|
|
||
| // Toggle the preference to view PDFs in the browser. | ||
| SetDownloadPDFs(false); | ||
| EXPECT_EQ(TestingCallbacks::WILL_ADD, pdf_extension_action()); | ||
| } | ||
|
|
||
| class BravePDFExtensionDisabledTest : public BravePDFExtensionTest { | ||
| public: | ||
| BravePDFExtensionDisabledTest() = default; | ||
| ~BravePDFExtensionDisabledTest() override = default; | ||
|
|
||
| protected: | ||
| void SetUpCommandLine(base::CommandLine* command_line) override { | ||
| ExtensionFunctionalTest::SetUpCommandLine(command_line); | ||
| // Disable loading of our PDF extension. | ||
| command_line->AppendSwitch(switches::kDisablePDFJSExtension); | ||
| } | ||
| }; | ||
|
|
||
| IN_PROC_BROWSER_TEST_F(BravePDFExtensionDisabledTest, ToggleDownloadPDFs) { | ||
| // Set preference to always download PDFs. | ||
| SetDownloadPDFs(true); | ||
| EXPECT_EQ(TestingCallbacks::WILL_REMOVE, pdf_extension_action()); | ||
|
|
||
| // Toggle the preference to view PDFs in the browser. | ||
| SetDownloadPDFs(false); | ||
| EXPECT_EQ(TestingCallbacks::WILL_REMOVE, pdf_extension_action()); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 changes: 16 additions & 0 deletions
16
chromium_src/extensions/common/manifest_handlers/mime_types_handler.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| /* This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this file, | ||
| * You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| #include "../../../../extensions/common/manifest_handlers/mime_types_handler.cc" | ||
|
|
||
| // static | ||
| std::vector<std::string> MimeTypesHandler::BraveGetMIMETypeWhitelist() { | ||
| std::vector<std::string> whitelist = MimeTypesHandler::GetMIMETypeWhitelist(); | ||
| auto pos = std::find(whitelist.begin(), whitelist.end(), | ||
| extension_misc::kPdfExtensionId); | ||
| if (pos != whitelist.end()) | ||
| whitelist.erase(pos); | ||
| return whitelist; | ||
| } | ||
|
|
Oops, something went wrong.