Show policy conflicts on chrome://policy page

Bug: 	930708
Change-Id: I0848524c1d637bd83cf40ddf2ac0402033ee1d5c
Reviewed-on: https://chromium-review.googlesource.com/c/1484496
Commit-Queue: Yann Dago <ydago@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636656}

chrome/browser/resources/policy/policy.css

@@ -52,11 +52,11 @@ body > main {
 
 .name {
   border-inline-end: 1px solid rgba(0, 0, 0, .06);
-  flex: 0 0 20%;
+  flex: 0 0 15%;
 }
 
 .value {
-  flex: 0 0 40%;
+  flex: 0 0 35%;
 }
 
 .row.header {
@@ -75,10 +75,9 @@ body > main {
   white-space: nowrap;
 }
 
-.expanded,
-.policy.row:hover {
+.expanded .policy.row:not(.conflict),
+.policy.row:not(.conflict):hover {
   background-color: rgb(250, 250, 250);
-  cursor: pointer;
 }
 
 .messages.row .value,
@@ -91,6 +90,7 @@ body > main {
 }
 
 .messages.row .name,
+.conflict.row .name,
 .value.row .name {
   text-align: end;
 }
@@ -141,12 +141,42 @@ body > header,
   padding: 12px;
 }
 
-.no-help-link .name-link {
+a {
+  color: rgb(26, 115, 232);
+  cursor: pointer;
+  text-decoration: underline;
+}
+
+.toggle {
+  cursor: pointer;
+}
+
+.name .link {
+  align-items: center;
+  display: flex;
+}
+
+.link span {
+  overflow: hidden;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+}
+
+.link img {
+  height: 12px;
+  width: 12px;
+}
+
+.no-help-link .name .link {
   color: inherit;
   pointer-events: none;
   text-decoration: none;
 }
 
+.no-help-link .link img {
+  display: none;
+}
+
 <if expr="android">
 .name {
   flex: 0 0 50%;

chrome/browser/resources/policy/policy.html

@@ -117,27 +117,53 @@
           <div class="source">$i18n{headerSource}</div>
           <div class="scope">$i18n{headerScope}</div>
           <div class="level">$i18n{headerLevel}</div>
-          <div class="messages">$i18n{headerWarning}</div>
+          <div class="messages">$i18n{headerStatus}</div>
+          <div class="toggle"></div>
         </div>
         <div class="no-policy">$i18n{noPoliciesSet}</div>
       </div>
     </div>
 
+    <div class="policy-conflict-data" id="policy-conflict-template" hidden>
+      <div class="policy conflict row">
+          <div class="name">$i18n{conflict}</div>
+          <div class="value"></div>
+          <div class="source"></div>
+          <div class="scope"></div>
+          <div class="level"></div>
+          <div class="messages"></div>
+          <div class="toggle"></div>
+      </div>
+      <div class="value row">
+        <div class="name"></div>
+        <div class="value"></div>
+      </div>
+    </div>
+
     <div class="policy-data" id="policy-template">
       <div class="policy row">
-        <div class="name"><a class="name-link"></a></div>
+        <div class="name">
+          <a class="link" target="_blank">
+            <span></span>
+            <img src="../../../../ui/webui/resources/images/open_in_new.svg">
+          </a>
+        </div>
         <div class="value"></div>
         <div class="source"></div>
         <div class="scope"></div>
         <div class="level"></div>
         <div class="messages"></div>
+        <div class="toggle">
+          <a is="action-link" class="show-more">$i18n{showMore}</a>
+          <a is="action-link" class="show-less" hidden>$i18n{showLess}</a>
+        </div>
       </div>
       <div class="value row" hidden>
         <div class="name">$i18n{value}</div>
         <div class="value"></div>
       </div>
       <div class="messages row" hidden>
-        <div class="name">$i18n{messages}</div>
+        <div class="name">$i18n{warning}</div>
         <div class="value"></div>
       </div>
     </div>

chrome/browser/resources/policy/policy_base.js

@@ -22,6 +22,16 @@ policy.PolicyNamesResponse;
  */
 policy.PolicyValuesResponse;
 
+/**
+ * @typedef {{
+ *    level: string,
+ *    scope: string,
+ *    source: string,
+ *    value: any,
+ * }}
+ */
+policy.Conflict;
+
 /**
  * @typedef {{
  *    name: string,
@@ -31,6 +41,7 @@ policy.PolicyValuesResponse;
  *    source: string,
  *    error: string,
  *    value: any,
+ *    conflicts: ?Array<!Conflict>,
  * }}
  */
 policy.Policy;
@@ -141,6 +152,39 @@ cr.define('policy', function() {
     },
   };
 
+  /**
+   * A single policy conflict's entry in the policy table.
+   * @constructor
+   * @extends {HTMLDivElement}
+   */
+  const PolicyConflict = cr.ui.define(function() {
+    const node = $('policy-conflict-template').cloneNode(true);
+    node.removeAttribute('id');
+    return node;
+  });
+
+  PolicyConflict.prototype = {
+    // Set up the prototype chain.
+    __proto__: HTMLDivElement.prototype,
+
+    decorate: function() {
+      this.querySelector('.policy.row')
+          .addEventListener('click', this.toggleExpanded_);
+    },
+
+    /** @param {Conflict} conflict */
+    initialize(conflict) {
+      this.querySelector('.scope').textContent = loadTimeData.getString(
+          conflict.scope == 'user' ? 'scopeUser' : 'scopeDevice');
+      this.querySelector('.level').textContent = loadTimeData.getString(
+          conflict.level == 'recommended' ? 'levelRecommended' :
+                                            'levelMandatory');
+      this.querySelector('.source').textContent =
+          loadTimeData.getString(conflict.source);
+      this.querySelector('.value.row .value').textContent = conflict.value;
+    }
+  };
+
   /**
    * A single policy's entry in the policy table.
    * @constructor
@@ -160,8 +204,8 @@ cr.define('policy', function() {
      * Initialization function for the cr.ui framework.
      */
     decorate: function() {
-      const policyRowDisplay = this.querySelector('.policy.row');
-      policyRowDisplay.addEventListener('click', this.toggleExpanded_);
+      const toggle = this.querySelector('.policy.row .toggle');
+      toggle.addEventListener('click', this.toggleExpanded_);
     },
 
     /** @param {Policy} policy */
@@ -175,13 +219,16 @@ cr.define('policy', function() {
       /** @private {boolean} */
       this.hasMessages_ = !!policy.error;
 
+      /** @private {boolean} */
+      this.hasConflicts_ = !!policy.conflicts;
+
       // Populate the name column.
-      const nameDisplay = this.querySelector('.name-link');
+      const nameDisplay = this.querySelector('.name .link span');
       nameDisplay.textContent = policy.name;
       if (policy.link) {
-        nameDisplay.href = policy.link;
-        nameDisplay.title =
-            loadTimeData.getStringF('policyLearnMore', policy.name);
+        const link = this.querySelector('.name .link');
+        link.href = policy.link;
+        link.title = loadTimeData.getStringF('policyLearnMore', policy.name);
       } else {
         this.classList.add('no-help-link');
       }
@@ -209,10 +256,6 @@ cr.define('policy', function() {
         const valueDisplay = this.querySelector('.value');
         valueDisplay.textContent = truncatedValue;
 
-        const messagesDisplay = this.querySelector('.messages');
-        messagesDisplay.textContent =
-            this.hasMessages_ ? loadTimeData.getString('messages') : '';
-        messagesDisplay.hidden = !this.hasMessages_;
 
         const valueRowContentDisplay = this.querySelector('.value.row .value');
         valueRowContentDisplay.textContent = policy.value;
@@ -221,13 +264,37 @@ cr.define('policy', function() {
             this.querySelector('.messages.row .value');
         messageRowContentDisplay.textContent = policy.error;
 
+        const messagesDisplay = this.querySelector('.messages');
+        const messagesNotice =
+            this.hasMessages_ ? loadTimeData.getString('warning') : '';
+        const conflictsNotice =
+            this.hasConflicts_ ? loadTimeData.getString('conflict') : '';
+        const notice = (messagesNotice && conflictsNotice) ?
+            loadTimeData.getString('warningAndConflicts') :
+            messagesNotice || conflictsNotice || loadTimeData.getString('ok');
+        messagesDisplay.textContent = notice;
+
         // <if expr="android">
         const valueRowDisplay = this.querySelector('.value.row');
         valueRowDisplay.hidden = false;
         valueDisplay.hidden = true;
         levelDisplay.hidden = true;
         scopeDisplay.hidden = true;
+        this.querySelector('.toggle').hidden = true;
+        row.querySelectorAll('.policy-conflict-data')
+            .forEach(row => row.hidden = false);
         // </if>
+
+        if (policy.conflicts) {
+          policy.conflicts.forEach(conflict => {
+            const row = new PolicyConflict;
+            row.initialize(conflict);
+            this.appendChild(row);
+          });
+        }
+      } else {
+        const messagesDisplay = this.querySelector('.messages');
+        messagesDisplay.textContent = loadTimeData.getString('unset');
       }
     },
 
@@ -237,13 +304,24 @@ cr.define('policy', function() {
      */
     toggleExpanded_: function() {
       // <if expr="not android">
-      const row = this.parentElement;
+      const row = this.parentElement.parentElement;
       const messageRowDisplay = row.querySelector('.messages.row');
       const valueRowDisplay = row.querySelector('.value.row');
       valueRowDisplay.hidden = !valueRowDisplay.hidden;
-      if (row.hasMessages) {
+      if (valueRowDisplay.hidden) {
+        row.classList.remove('expanded');
+      } else {
+        row.classList.add('expanded');
+      }
+
+      const messagesDisplay = row.querySelector('.messages');
+      this.querySelector('.show-more').hidden = !valueRowDisplay.hidden;
+      this.querySelector('.show-less').hidden = valueRowDisplay.hidden;
+      if (messagesDisplay.textContent !== loadTimeData.getString('ok')) {
         messageRowDisplay.hidden = !messageRowDisplay.hidden;
       }
+      row.querySelectorAll('.policy-conflict-data')
+          .forEach(row => row.hidden = !row.hidden);
       // </if>
     },
   };
@@ -276,7 +354,7 @@ cr.define('policy', function() {
     update(dataModel) {
       // Clear policies
       const mainContent = this.querySelector('.main');
-      const policies = this.querySelectorAll('.policies-data');
+      const policies = this.querySelectorAll('.policy-data');
       this.querySelector('.header').textContent = dataModel.name;
       this.querySelector('.id').textContent = dataModel.id;
       this.querySelector('.id').hidden = !dataModel.id;

chrome/browser/ui/webui/policy_ui.cc

@@ -54,10 +54,8 @@ content::WebUIDataSource* CreatePolicyUIHtmlSource() {
   source->AddLocalizedString("labelStatus", IDS_POLICY_LABEL_STATUS);
   source->AddLocalizedString("showUnset", IDS_POLICY_SHOW_UNSET);
   source->AddLocalizedString("noPoliciesSet", IDS_POLICY_NO_POLICIES_SET);
-  source->AddLocalizedString("showExpandedValue",
-                             IDS_POLICY_SHOW_EXPANDED_VALUE);
-  source->AddLocalizedString("hideExpandedValue",
-                             IDS_POLICY_HIDE_EXPANDED_VALUE);
+  source->AddLocalizedString("showMore", IDS_POLICY_SHOW_MORE);
+  source->AddLocalizedString("showLess", IDS_POLICY_SHOW_LESS);
   source->AddLocalizedString("showExpandedStatus",
                              IDS_POLICY_SHOW_EXPANDED_STATUS);
   source->AddLocalizedString("hideExpandedStatus",

chrome/browser/ui/webui/policy_ui_browsertest.cc

@@ -137,10 +137,11 @@ std::vector<std::string> PopulateExpectedPolicy(
   // Populate expected status.
   if (unknown)
     expected_policy.push_back(
-        l10n_util::GetStringUTF8(IDS_POLICY_LABEL_MESSAGES));
+        l10n_util::GetStringUTF8(IDS_POLICY_HEADER_WARNING));
+  else if (!policy_map_entry)
+    expected_policy.push_back(l10n_util::GetStringUTF8(IDS_POLICY_UNSET));
   else
-    expected_policy.push_back(std::string());
-
+    expected_policy.push_back(l10n_util::GetStringUTF8(IDS_POLICY_OK));
   return expected_policy;
 }
 
@@ -276,7 +277,7 @@ void PolicyUITest::VerifyPolicies(
       "  for (var j = 0; j < items.length; ++j) {"
       "    var children = items[j].querySelectorAll('div');"
       "    var values = [];"
-      "    for(var k = 0; k < children.length; ++k) {"
+      "    for(var k = 0; k < children.length - 1; ++k) {"
       "      values.push(children[k].textContent.trim());"
       "    }"
       "    policies.push(values);"
@@ -304,7 +305,8 @@ void PolicyUITest::VerifyPolicies(
     for (size_t j = 0; j < expected_policy.size(); ++j) {
       std::string value;
       ASSERT_TRUE(actual_policy->GetString(j, &value));
-      EXPECT_EQ(expected_policy[j], value);
+      if (expected_policy[j] != value)
+        EXPECT_EQ(expected_policy[j], value);
     }
   }
 }

chrome/browser/ui/webui/policy_ui_handler.cc

@@ -624,16 +624,19 @@ void PolicyUIHandler::AddCommonLocalizedStringsToSource(
       content::WebUIDataSource* source) {
   AddLocalizedPolicyStrings(source, policy::kPolicySources,
                             static_cast<size_t>(policy::POLICY_SOURCE_COUNT));
+  source->AddLocalizedString("conflict", IDS_POLICY_LABEL_CONFLICT);
   source->AddLocalizedString("headerLevel", IDS_POLICY_HEADER_LEVEL);
   source->AddLocalizedString("headerName", IDS_POLICY_HEADER_NAME);
   source->AddLocalizedString("headerScope", IDS_POLICY_HEADER_SCOPE);
   source->AddLocalizedString("headerSource", IDS_POLICY_HEADER_SOURCE);
   source->AddLocalizedString("headerStatus", IDS_POLICY_HEADER_STATUS);
   source->AddLocalizedString("headerValue", IDS_POLICY_HEADER_VALUE);
-  source->AddLocalizedString("headerWarning", IDS_POLICY_HEADER_WARNING);
+  source->AddLocalizedString("warning", IDS_POLICY_HEADER_WARNING);
   source->AddLocalizedString("levelMandatory", IDS_POLICY_LEVEL_MANDATORY);
   source->AddLocalizedString("levelRecommended", IDS_POLICY_LEVEL_RECOMMENDED);
   source->AddLocalizedString("messages", IDS_POLICY_LABEL_MESSAGES);
+  source->AddLocalizedString("warningAndConflicts",
+                             IDS_POLICY_LABEL_WARNING_AND_CONFLICT);
   source->AddLocalizedString("notSpecified", IDS_POLICY_NOT_SPECIFIED);
   source->AddLocalizedString("ok", IDS_POLICY_OK);
   source->AddLocalizedString("scopeDevice", IDS_POLICY_SCOPE_DEVICE);

components/policy_strings.grdp

@@ -322,6 +322,12 @@ Additional details:
   <message name="IDS_POLICY_LABEL_MESSAGES" desc="Label for the messages row in the policy table.">
     Messages
   </message>
+  <message name="IDS_POLICY_LABEL_CONFLICT" desc="Label for the conflict row in the policy table.">
+    Conflict
+  </message>
+  <message name="IDS_POLICY_LABEL_WARNING_AND_CONFLICT" desc="Label for the conflict row in the policy table when there are warnings and conflicts.">
+    Warnings, Conflict
+  </message>
   <message name="IDS_POLICY_LABEL_VALUE" desc="Label for the value row in the policy table.">
     Value
   </message>
@@ -355,11 +361,11 @@ Additional details:
   <message name="IDS_POLICY_HEADER_WARNING" desc="Table header for the column in the policy table that contains the policy warnings.">
     Warning
   </message>
-  <message name="IDS_POLICY_SHOW_EXPANDED_VALUE" desc="Text for the link that shows the policy value. Used when the policy value is too long to be always visible.">
-    Show value
+  <message name="IDS_POLICY_SHOW_MORE" desc="Text for the link that expands and shows the are full value, conflicts and warnings rows of a policy.">
+    Show more
   </message>
-  <message name="IDS_POLICY_HIDE_EXPANDED_VALUE" desc="Text for the link that hides the policy value. Used when the policy value is too long to be always visible.">
-    Hide value
+  <message name="IDS_POLICY_SHOW_LESS" desc="Text for the link that collapses and hides the are full value, conflicts and warnings rows of a policy.">
+    Show less
   </message>
   <message name="IDS_POLICY_LEARN_MORE" desc="Help text for learn-more link for known chrome policies.">
     Learn more about <ph name="POLICY_NAME">$1<ex>AllowDinosaurEasterEgg</ex></ph> policy