Fix display and progressive disclosure for multi-line values in about:policy.

Previously, all lines were joined together into a single hard-to-read
block. Change expanded state to make white-space significant so
multi-line error messages are on separate lines and network
configuration renders nicely.

BUG=chromium:104138
TEST=Put network configuration policy in place and check about:policy.


Review URL: http://codereview.chromium.org/8555009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@110070 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 88 insertions, 75 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 5d1935c..9b9b12e 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -4394,10 +4394,10 @@ Keep your key file in a safe place. You will need it to create new versions of y
         Status
       </message>
       <message name="IDS_POLICY_SHOW_MORE" desc="The text for the link that expands a policy value or policy status table cell.">
-        [Show more]
+        Show more
       </message>
       <message name="IDS_POLICY_HIDE" desc="The text for the link that hides overflowing text in a policy value or policy status table cell.">
-        [Hide]
+        Hide
       </message>
       <message name="IDS_POLICY_NEVER_FETCHED" desc="Indicates that a policy fetch was never performed before.">
         Never
diff --git a/chrome/browser/policy/policy_error_map.cc b/chrome/browser/policy/policy_error_map.cc
index 956392a..daf167a 100644
--- a/chrome/browser/policy/policy_error_map.cc
+++ b/chrome/browser/policy/policy_error_map.cc
@@ -57,7 +57,7 @@ string16 PolicyErrorMap::GetErrors(ConfigurationPolicyType policy) {
   std::vector<string16> list;
   for (const_iterator it = range.first; it != range.second; ++it)
     list.push_back(it->second);
-  return JoinString(list, ' ');
+  return JoinString(list, '\n');
 }
 
 bool PolicyErrorMap::empty() {
diff --git a/chrome/browser/resources/policy.css b/chrome/browser/resources/policy.css
index ba43634..147b949 100644
--- a/chrome/browser/resources/policy.css
+++ b/chrome/browser/resources/policy.css
@@ -136,24 +136,44 @@ legend {
 
 #policy-table td {
   background-color: #eaeef3;
-  overflow: hidden;
   padding: 10px;
-  text-overflow: ellipsis;
+  position: relative;
+  vertical-align: top;
   width: 20%;
 }
 
-.toggler {
-  color: #808080;
-  cursor: pointer;
+.text-collapsed {
+  bottom: 0;
+  left: 0;
+  margin: 10px;
+  overflow: hidden;
+  position: absolute;
+  right: 0;
+  top: 0;
 }
 
-.collapsed {
+.text-cell {
   overflow: hidden;
+}
+
+.text-collapsed .text-cell {
   text-overflow: ellipsis;
-  white-space: nowrap;
 }
 
-.expanded {
+.text-expanded .text-cell {
   overflow: visible;
   word-wrap: break-word;
 }
+
+.text-collapsed .text-value {
+  white-space: nowrap;
+}
+
+.text-expanded .text-value {
+  white-space: pre-wrap;
+}
+
+.toggler {
+  float: right;
+  padding: 0 3px !important;
+}
diff --git a/chrome/browser/resources/policy.html b/chrome/browser/resources/policy.html
index 8f7349e..e904b46 100644
--- a/chrome/browser/resources/policy.html
+++ b/chrome/browser/resources/policy.html
@@ -127,24 +127,26 @@
                   <span class="policy-name" jscontent="name"></span>
                 </td>
                 <td>
-                  <div class="text-container collapsed">
-                    <span class="cell-text" jscontent="value"></span>
+                  <div class="text-collapsed text-container">
+                    <button class="link-button toggler expand"
+                        i18n-content="showMoreText" hidden></button>
+                    <div class="text-cell">
+                      <span class="text-value" jscontent="value"></span>
+                    </div>
+                    <button class="link-button toggler collapse"
+                        i18n-content="hideText" hidden></button>
                   </div>
-                  <a class="toggler expand" style="display: none"
-                      jsdisplay="Policy.shouldShowExpand(this)"
-                      i18n-content="showMoreText"></a>
-                  <a class="toggler collapse" style="display: none"
-                      i18n-content="hideText"></a>
                 </td>
                 <td>
-                  <div class="text-container collapsed">
-                    <span class="cell-text" jscontent="status"></span>
+                  <div class="text-collapsed text-container">
+                    <button class="link-button toggler expand"
+                        i18n-content="showMoreText" hidden></button>
+                    <div class="text-cell">
+                      <span class="text-value" jscontent="status"></span>
+                    </div>
+                    <button class="link-button toggler collapse"
+                        i18n-content="hideText" hidden></button>
                   </div>
-                  <a class="toggler expand" style="display: none"
-                      jsdisplay="Policy.shouldShowExpand(this)"
-                      i18n-content="showMoreText"></a>
-                  <a class="toggler collapse" style="display: none"
-                      i18n-content="hideText"></a>
                 </td>
               </tr>
             </table>
diff --git a/chrome/browser/resources/policy.js b/chrome/browser/resources/policy.js
index eb564cd..8fad182 100644
--- a/chrome/browser/resources/policy.js
+++ b/chrome/browser/resources/policy.js
@@ -79,7 +79,18 @@ cr.define('policies', function() {
       var input = new JsEvalContext(policyData);
       var output = $('data-template');
       jstProcess(input, output);
-      this.setToggleEventHandlers_();
+
+      var toggles = document.querySelectorAll('.policy-set * .toggler');
+      for (var i = 0; i < toggles.length; i++) {
+        toggles[i].hidden = true;
+        toggles[i].onclick = function() {
+          Policy.getInstance().toggleCellExpand_(this);
+        };
+      }
+
+      var containers = document.querySelectorAll('.text-container');
+      for (var i = 0; i < containers.length; i++)
+        this.initTextContainer_(containers[i])
     },
 
     /**
@@ -124,38 +135,18 @@ cr.define('policies', function() {
     },
 
     /**
-     * Set event handlers for the "Show more"/"Hide" links generated by
-     * jstemplate.
-     * @private
-     */
-    setToggleEventHandlers_: function() {
-      var toggles = document.querySelectorAll('.policy-set * .toggler');
-      for (var i = 0; i < toggles.length; i++) {
-        toggles[i].onclick = function() {
-          Policy.getInstance().toggleCellExpand_(this);
-        };
-      }
-    },
-
-    /**
      * Expands or collapses a table cell that has overflowing text.
      * @param {Object} toggler The toggler that was clicked on.
      * @private
      */
     toggleCellExpand_: function(toggler) {
-      var tableCell = toggler.parentElement;
-      var textContainer = tableCell.querySelector('.text-container');
+      var textContainer = toggler.parentElement;
+      textContainer.collapsed = !textContainer.collapsed;
 
       if (textContainer.collapsed)
-        this.expandCell_(textContainer);
-      else
         this.collapseCell_(textContainer);
-
-      textContainer.collapsed = !textContainer.collapsed;
-      var expand = tableCell.querySelector('.expand');
-      var collapse = tableCell.querySelector('.collapse');
-      expand.style.display = textContainer.collapsed ? '' : 'none';
-      collapse.style.display = textContainer.collapsed ? 'none' : '';
+      else
+        this.expandCell_(textContainer);
     },
 
     /**
@@ -164,11 +155,7 @@ cr.define('policies', function() {
      * the table is updated.
      */
     collapseExpandedCells: function() {
-      var toggles = document.querySelectorAll('.policy-set * .toggler');
-      for (var i = 0; i < toggles.length; i++)
-        toggles[i].style.display = 'none';
-
-      var textContainers = document.querySelectorAll('.expanded');
+      var textContainers = document.querySelectorAll('.text-expanded');
       for (var i = 0; i < textContainers.length; i++)
         this.collapseCell_(textContainers[i]);
     },
@@ -180,8 +167,10 @@ cr.define('policies', function() {
      * @private
      */
     expandCell_: function(textContainer) {
-      textContainer.classList.remove('collapsed');
-      textContainer.classList.add('expanded');
+      textContainer.classList.remove('text-collapsed');
+      textContainer.classList.add('text-expanded');
+      textContainer.querySelector('.expand').hidden = true;
+      textContainer.querySelector('.collapse').hidden = false;
     },
 
     /**
@@ -191,8 +180,26 @@ cr.define('policies', function() {
      * @private
      */
     collapseCell_: function(textContainer) {
-      textContainer.classList.remove('expanded');
-      textContainer.classList.add('collapsed');
+      textContainer.classList.remove('text-expanded');
+      textContainer.classList.add('text-collapsed');
+      textContainer.querySelector('.expand').hidden = false;
+      textContainer.querySelector('.collapse').hidden = true;
+    },
+
+    /**
+     * Initializes a text container, showing the expand toggle if necessary.
+     * @param {Object} textContainer The text container element.
+     */
+    initTextContainer_: function(textContainer) {
+      textContainer.collapsed = true;
+      var textValue = textContainer.querySelector('.text-value');
+
+      // If the text is wider than the text container, the expand toggler should
+      // appear.
+      if (textContainer.offsetWidth < textValue.offsetWidth ||
+          textContainer.offsetHeight < textValue.offsetHeight) {
+        this.collapseCell_(textContainer);
+      }
     }
   };
 
@@ -238,22 +245,6 @@ cr.define('policies', function() {
   };
 
   /**
-   * Returns true if the "Show more" toggle should appear in a table cell and
-   * false if not.
-   * @param {Object} expandToggle The "Show more" DOM element.
-   */
-  Policy.shouldShowExpand = function(expandToggle) {
-    var textContainer =
-        expandToggle.parentNode.querySelector('.text-container');
-    textContainer.collapsed = true;
-    var cellText = textContainer.querySelector('.cell-text');
-
-    // If the text is wider than the text container, the expand toggler should
-    // appear.
-    return textContainer.offsetWidth < cellText.offsetWidth;
-  };
-
-  /**
    * Initializes the page and loads the list of policies and the policy
    * status data.
    */