Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(226)

Issue 2392923002: Add UI for local manipulation of the PDF plugin preference. (Closed)

Created:
4 years, 2 months ago by pastarmovj
Modified:
4 years, 2 months ago
Reviewers:
Dan Beam
CC:
chromium-reviews, dbeam+watch-options_chromium.org, michaelpg+watch-options_chromium.org, arv+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add UI for local manipulation of the PDF plugin preference. It shows the state of the pref in the content settings dialog and allows its modification if not controlled by policy in which case an idicator is present to make this visible. Known issues that will be addressed in future CLs - On about:plugins the plugin is shown as disbled by policy even if disabled by the user. - If Chrome is default app for PDFs and this setting is on a vicious loop will happen where Chrome will copy the file over and over. Downloads should not proceed to opening PDFs in this case regardless of the user's choice. BUG=628014 TEST=automatic ui tests. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/8c7be7eaa985a8b4a29de3dda25963f4f2f12c86 Cr-Commit-Position: refs/heads/master@{#423268}

Patch Set 1 #

Patch Set 2 : Clean up the html and add tests. #

Patch Set 3 : Rebased on a cleaned up base CL. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -1 line) Patch
M chrome/app/generated_resources.grd View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/content_settings.html View 1 1 chunk +17 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/content_settings_handler.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/test/data/policy/policy_test_cases.json View 1 1 chunk +6 lines, -1 line 0 comments Download

Depends on Patchset:

Messages

Total messages: 11 (5 generated)
pastarmovj
Hi Steven, please review this CL. Thanks, Julian
4 years, 2 months ago (2016-10-04 15:27:02 UTC) #3
pastarmovj
Steven is ooo, Dan, can you please take a look?
4 years, 2 months ago (2016-10-05 08:24:05 UTC) #5
Dan Beam
lgtm
4 years, 2 months ago (2016-10-05 17:26:33 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2392923002/40001
4 years, 2 months ago (2016-10-05 18:02:54 UTC) #8
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 2 months ago (2016-10-05 20:07:39 UTC) #9
commit-bot: I haz the power
4 years, 2 months ago (2016-10-05 20:11:23 UTC) #11
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/8c7be7eaa985a8b4a29de3dda25963f4f2f12c86
Cr-Commit-Position: refs/heads/master@{#423268}

Powered by Google App Engine
This is Rietveld 408576698