Skip to content

Commit

Permalink
Allow downloading PDFs instead of opening them in Brave.
Browse files Browse the repository at this point in the history
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.

2. Watches profile preference value kPluginsAlwaysOpenPdfExternally and
adds/removes PDFJS extension 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.

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.
  • Loading branch information
mkarolin committed Nov 20, 2018
1 parent 838d82d commit 70f394a
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 3 deletions.
45 changes: 43 additions & 2 deletions browser/extensions/brave_component_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,33 @@
#include "brave/components/brave_rewards/browser/buildflags/buildflags.h"
#include "brave/components/brave_rewards/resources/extension/grit/brave_rewards_extension_resources.h"
#include "brave/components/brave_webtorrent/grit/brave_webtorrent_resources.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/pref_names.h"
#include "components/grit/brave_components_resources.h"
#include "extensions/browser/extension_prefs.h"

namespace extensions {

//static
bool BraveComponentLoader::IsPdfjsDisabled() {
const base::CommandLine& command_line =
*base::CommandLine::ForCurrentProcess();
return command_line.HasSwitch(switches::kDisablePDFJSExtension);
}

BraveComponentLoader::BraveComponentLoader(
ExtensionServiceInterface* extension_service,
PrefService* profile_prefs,
PrefService* local_state,
Profile* profile)
: ComponentLoader(extension_service, profile_prefs, local_state, profile),
profile_(profile) {
profile_(profile),
profile_prefs_(profile_prefs),
always_open_pdf_externally_(false) {
DCHECK(profile_prefs_);
always_open_pdf_externally_ =
profile_prefs_->GetBoolean(prefs::kPluginsAlwaysOpenPdfExternally);
ObserveOpenPdfExternallySetting();
}

BraveComponentLoader::~BraveComponentLoader() {
Expand Down Expand Up @@ -73,7 +88,8 @@ void BraveComponentLoader::AddDefaultComponentExtensions(
Add(IDR_BRAVE_EXTENSON, brave_extension_path);
}

if (!command_line.HasSwitch(switches::kDisablePDFJSExtension)) {
if (!always_open_pdf_externally_ &&
!command_line.HasSwitch(switches::kDisablePDFJSExtension)) {
AddExtension(pdfjs_extension_id, pdfjs_extension_name, pdfjs_extension_public_key);
}

Expand All @@ -94,4 +110,29 @@ void BraveComponentLoader::AddDefaultComponentExtensions(
}
}

void BraveComponentLoader::ObserveOpenPdfExternallySetting() {
// Observe the setting change only in regular profiles since the PDF settings
// page is not available in Guest/Tor profiles.
DCHECK(profile_ && profile_prefs_);
if (!profile_->IsGuestSession()) {
registrar_.Init(profile_prefs_);
registrar_.Add(prefs::kPluginsAlwaysOpenPdfExternally,
base::Bind(&BraveComponentLoader::UpdatePdfExtension,
base::Unretained(this)));
}
}

void BraveComponentLoader::UpdatePdfExtension(const std::string& pref_name) {
DCHECK(pref_name == prefs::kPluginsAlwaysOpenPdfExternally);
DCHECK(profile_prefs_);
if (!profile_prefs_->GetBoolean(prefs::kPluginsAlwaysOpenPdfExternally)) {
if (!Exists(pdfjs_extension_id)) {
AddExtension(pdfjs_extension_id, pdfjs_extension_name,
pdfjs_extension_public_key);
}
} else {
Remove(pdfjs_extension_id);
}
}

} // namespace extensions
10 changes: 10 additions & 0 deletions browser/extensions/brave_component_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "base/files/file_path.h"
#include "chrome/browser/extensions/component_loader.h"
#include "components/prefs/pref_change_registrar.h"

namespace extensions {

Expand All @@ -32,8 +33,17 @@ class BraveComponentLoader : public ComponentLoader {
void AddExtension(const std::string& id,
const std::string& name, const std::string& public_key);

static bool IsPdfjsDisabled();

private:
void ObserveOpenPdfExternallySetting();
// Callback for changes to the AlwaysOpenPdfExternally setting.
void UpdatePdfExtension(const std::string& pref_name);

Profile* profile_;
PrefService* profile_prefs_;
PrefChangeRegistrar registrar_;
bool always_open_pdf_externally_;
DISALLOW_COPY_AND_ASSIGN(BraveComponentLoader);
};

Expand Down
6 changes: 5 additions & 1 deletion browser/ui/webui/brave_md_settings_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "brave/browser/ui/webui/brave_md_settings_ui.h"

#include "brave/browser/extensions/brave_component_loader.h"
#include "brave/browser/resources/grit/brave_settings_resources.h"
#include "brave/browser/resources/grit/brave_settings_resources_map.h"
#include "brave/browser/ui/webui/settings/brave_privacy_handler.h"
Expand All @@ -26,8 +27,11 @@ BraveMdSettingsUI::~BraveMdSettingsUI() {
// static
void BraveMdSettingsUI::AddResources(content::WebUIDataSource* html_source,
Profile* profile) {
for (size_t i = 0; i < kBraveSettingsResourcesSize; ++i) {
for (size_t i = 0; i < kBraveSettingsResourcesSize; ++i) {
html_source->AddResourcePath(kBraveSettingsResources[i].name,
kBraveSettingsResources[i].value);
}

html_source->AddBoolean("isPdfjsDisabled",
extensions::BraveComponentLoader::IsPdfjsDisabled());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
diff --git a/chrome/browser/resources/settings/site_settings/pdf_documents.js b/chrome/browser/resources/settings/site_settings/pdf_documents.js
index 66f61a6db59bc5a549db3ec505518462f72b5cb7..dc96ac708d68be5f82972aa3273e38ce687b61ab 100644
--- a/chrome/browser/resources/settings/site_settings/pdf_documents.js
+++ b/chrome/browser/resources/settings/site_settings/pdf_documents.js
@@ -17,4 +17,12 @@ Polymer({
notify: true,
},
},
+
+ /** @override */
+ ready: function () {
+ if (loadTimeData.getBoolean('isPdfjsDisabled')) {
+ this.$.toggle.disabled = true;
+ this.$.toggle.checked = true;
+ }
+ },
});

0 comments on commit 70f394a

Please sign in to comment.