MD Settings: About: Show connectionTypes message

chrome/browser/resources/settings/about_page/about_page.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 <link rel="import" href="about_page_browser_proxy.html">
 <link rel="import" href="../i18n_setup.html">
 <link rel="import" href="../icons.html">
 <link rel="import" href="../lifetime_browser_proxy.html">
 <link rel="import" href="../route.html">
 <link rel="import" href="../settings_page/main_page_behavior.html">
 <link rel="import" href="../settings_page/settings_animated_pages.html">
 <link rel="import" href="../settings_page/settings_section.html">
 <link rel="import" href="../settings_page_css.html">
@@ skipping 81 matching lines @@
             </div>
             <div class="settings-box two-line">
               <iron-icon
                   hidden="[[!showUpdateStatus_]]"
                   icon$="[[getIcon_(
                       obsoleteSystemInfo_, currentUpdateStatusEvent_)]]"
                   src="[[getIconSrc_(
                       obsoleteSystemInfo_, currentUpdateStatusEvent_)]]">
               </iron-icon>
               <div class="start">
+<if expr="not chromeos">
                 <div id="updateStatusMessage" hidden="[[!showUpdateStatus_]]"
-<if expr="not chromeos">
                     inner-h-t-m-l="[[getUpdateStatusMessage_(
                         currentUpdateStatusEvent_)]]">
+                </div>
 </if>
 <if expr="chromeos">
+                <div id="updateStatusMessage" hidden="[[!showUpdateStatus_]]"
                     inner-h-t-m-l="[[getUpdateStatusMessage_(
                         currentUpdateStatusEvent_, targetChannel_)]]">
+                </div>
+                <div id="updateStatusMessage" hidden="[[!showConnectionType_]]"

[dpapad, 2017/02/16 21:22:27]

Having two elements with the same ID in the DOM does not seem right. For example
which one is returned by this.$.updateStatusMessage?

Can you instead update the getUpdateStatusMessage_ function to return the
correct string? It already has ChromeOS specific logic inside it, and it already
depends on the currentUpdateStatusEvent_.

Also should we be updating the tests at
https://cs.chromium.org/chromium/src/chrome/test/data/webui/settings/about_pa...

[stevenjb, 2017/02/17 01:06:01]

Oops. Copy/paste bug. I'll change the id and add a test.

I don't understand your "Can you instead " suggestion. Are you suggesting
embedding multiple divs in the result? I wouldn't be super fond of that
approach, and testing becomes more fragile.

[dpapad, 2017/02/17 01:14:38]

Conceptually the connection types error message is part of the "update status
message" and getUpdateStatusMessage_ already returns HTML code and interpreted
as innerHTML. What justifies having the connection types related message as a
separate message instead of including it in the return value of
getUpdateStatusMessage_()?
Regarding testing, asserting that this.$.updateStatusMessage.textContent
contains some substring expected to exist in the new error message seems solid
enough, no?

[stevenjb, 2017/02/17 01:28:51]

I feel like there is a difference between embedding strings that may include <b>
tags and generating divs, but it's hardly worth depating as far as I am
concerned so I will just change it.

[dpapad, 2017/02/17 02:44:28]

FWIW, the <b> is just a trick. We don't actually include it in the DOM
(firstChild.innerHTML is getting the contents of <b>).

+                    inner-h-t-m-l="[[
+                        currentUpdateStatusEvent_.connectionTypes]]">
+                </div>
 </if>
-                </div>
                 <span id="deprecationWarning"
                     hidden="[[!obsoleteSystemInfo_.obsolete]]">
                   $i18n{aboutObsoleteSystem}
                   <a href="$i18n{aboutObsoleteSystemURL}" target="_blank">
                     $i18n{learnMore}
                   </a>
                 </span>
                 <div class="secondary copyable">$i18n{aboutBrowserVersion}</div>
               </div>
               <span id="buttonContainer" class="secondary-action"
@@ skipping 89 matching lines @@
               <settings-detailed-build-info></settings-detailed-build-info>
             </settings-subpage>
           </template>
 </if>
         </settings-animated-pages>
       </settings-section>
     </div>
   </template>
   <script src="about_page.js"></script>
 </dom-module>