New issue
Advanced search Search tips

Issue 647399 link

Starred by 3 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 656758

Blocking:
issue 712446
issue 753929



Sign in to add a comment

[Site Settings Details] Find a way to check for new categories in handleGetSiteDetails

Project Member Reported by dschuyler@chromium.org, Sep 15 2016

Issue description

The the CL 
https://codereview.chromium.org/2338163004/diff/20001/chrome/browser/ui/webui/settings/site_settings_handler.cc?context=10&column_width=80&tab_spaces=8

Follow up on

On 2016/09/15 12:51:11, Finnur wrote:
> Can we somehow enforce (perhaps just with a test) that this does not go out of
> sync when a new category is added to the Site Settings UI?


 
Summary: [Site Settings Details] Find a way to check for new categories in handleGetSiteDetails (was: Find a way to check for new categories in handleGetSiteDetails)
Blocking: 656758
Blocking: 712446
Status: Available (was: Assigned)
Blocking: -656758
Blockedon: 656758
Owner: ----
Blocking: 753929
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb5561268c278510a39f2090aa20da2686989136

commit bb5561268c278510a39f2090aa20da2686989136
Author: Patti <patricialor@chromium.org>
Date: Fri Aug 25 00:30:58 2017

MD Settings: Site Details now shows protected content setting on ChromeOS.

The Protected Media Identifier / Protected Content setting is missing from Site
Details. Add it.

Bug:  656758 , 647399
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I03078200f3533d6271c8fd76bcb53f2e3dbb9833
Reviewed-on: https://chromium-review.googlesource.com/628242
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497257}
[modify] https://crrev.com/bb5561268c278510a39f2090aa20da2686989136/chrome/browser/resources/settings/site_settings/site_details.html
[modify] https://crrev.com/bb5561268c278510a39f2090aa20da2686989136/chrome/test/data/webui/settings/site_details_tests.js
[modify] https://crrev.com/bb5561268c278510a39f2090aa20da2686989136/chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f2232ec42931da29ea04eb9be3657117b49f8fc

commit 3f2232ec42931da29ea04eb9be3657117b49f8fc
Author: Yoichi Osato <yoichio@chromium.org>
Date: Fri Aug 25 05:51:25 2017

Revert "MD Settings: Site Details now shows protected content setting on ChromeOS."

This reverts commit bb5561268c278510a39f2090aa20da2686989136.

Reason for revert: Findit found culprit 497257 with 96% for linux test failure:
confidencehttps://uberchromegw.corp.google.com/i/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29

Original change's description:
> MD Settings: Site Details now shows protected content setting on ChromeOS.
> 
> The Protected Media Identifier / Protected Content setting is missing from Site
> Details. Add it.
> 
> Bug:  656758 , 647399
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: I03078200f3533d6271c8fd76bcb53f2e3dbb9833
> Reviewed-on: https://chromium-review.googlesource.com/628242
> Commit-Queue: Patti <patricialor@chromium.org>
> Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#497257}

TBR=dschuyler@chromium.org,patricialor@chromium.org

Change-Id: Id7b9771b6b83805b6d88c7a8ae7da6140afaf5e2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  656758 , 647399
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/634883
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497331}
[modify] https://crrev.com/3f2232ec42931da29ea04eb9be3657117b49f8fc/chrome/browser/resources/settings/site_settings/site_details.html
[modify] https://crrev.com/3f2232ec42931da29ea04eb9be3657117b49f8fc/chrome/test/data/webui/settings/site_details_tests.js
[modify] https://crrev.com/3f2232ec42931da29ea04eb9be3657117b49f8fc/chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80dac7ffe6771d2a32586f3abf1e02ebad94840b

commit 80dac7ffe6771d2a32586f3abf1e02ebad94840b
Author: Patti <patricialor@chromium.org>
Date: Mon Sep 25 01:55:42 2017

MD Settings: Site Details now shows protected content setting on ChromeOS.

The Protected Media Identifier / Protected Content setting is missing from Site
Details. Add it.

Bug:  656758 , 647399
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ieb458c5897a1628972effbf4cfabb7aaa69e9927
Reviewed-on: https://chromium-review.googlesource.com/641296
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503968}
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/app/settings_strings.grdp
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/resources/settings/route.js
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/resources/settings/settings_resources.grd
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/resources/settings/site_settings/compiled_resources2.gyp
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/resources/settings/site_settings/constants.js
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/resources/settings/site_settings/site_details.html
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/resources/settings/site_settings/site_details.js
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/resources/settings/site_settings/site_details_permission.html
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/resources/settings/site_settings/site_details_permission.js
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/resources/settings/site_settings_page/site_settings_page.js
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/ui/webui/settings/site_settings_handler.cc
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/ui/webui/settings/site_settings_handler.h
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/ui/webui/site_settings_helper.cc
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/ui/webui/site_settings_helper.h
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/browser/ui/webui/site_settings_helper_unittest.cc
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/test/data/webui/settings/route_tests.js
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/test/data/webui/settings/site_details_permission_tests.js
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/test/data/webui/settings/site_details_tests.js
[modify] https://crrev.com/80dac7ffe6771d2a32586f3abf1e02ebad94840b/chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d00ec628379069f37a5a85618430fce350f89c9

commit 9d00ec628379069f37a5a85618430fce350f89c9
Author: Patti <patricialor@chromium.org>
Date: Mon Oct 09 02:27:29 2017

MD Settings: Enforce new ContentSettingsTypes are synced to Site Settings.

Add a static_assert to make sure new ContentSettingsTypes can't be added without
considering whether they need corresponding UI in Site Settings
(chrome://settings/content). This will cause a compile failure until the new
ContentSettingsType owner adds an entry to
SiteSettingsHelper::kContentSettingsTypeGroupNames.

This patch does not enforce syncing of the Javascript permission category object
(settings.ContentSettingsTypes) in site_settings/constants.js.

Bug: 647399
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I26158e53b0a28ab59e760b5234a3540b79442a4a
Reviewed-on: https://chromium-review.googlesource.com/618535
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507319}
[modify] https://crrev.com/9d00ec628379069f37a5a85618430fce350f89c9/chrome/browser/resources/settings/site_settings/constants.js
[modify] https://crrev.com/9d00ec628379069f37a5a85618430fce350f89c9/chrome/browser/resources/settings/site_settings/site_details.html
[modify] https://crrev.com/9d00ec628379069f37a5a85618430fce350f89c9/chrome/browser/resources/settings/site_settings/site_details.js
[modify] https://crrev.com/9d00ec628379069f37a5a85618430fce350f89c9/chrome/browser/ui/webui/site_settings_helper.cc
[modify] https://crrev.com/9d00ec628379069f37a5a85618430fce350f89c9/chrome/test/data/webui/settings/site_details_tests.js

Project Member

Comment 13 by sheriffbot@chromium.org, Oct 9

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment