Viewing cached webpages while offline is not available on Chrome OS

chrome/renderer/resources/neterror.js

Index: chrome/renderer/resources/neterror.js
diff --git a/chrome/renderer/resources/neterror.js b/chrome/renderer/resources/neterror.js
index 360d0b840c2b73591e5cc850e4972828a0963544..81de561b5d849c1b5ab3562a74f21ac544661845 100644
--- a/chrome/renderer/resources/neterror.js
+++ b/chrome/renderer/resources/neterror.js
@@ -131,12 +131,14 @@ function onDocumentLoad() {
     detailsButton.classList.add('singular');
   }
 
+<if expr="not chromeos">
   // Hide the details button if there are no details to show.
   if (loadTimeData.valueExists('summary') &&
           !loadTimeData.getValue('summary').msg) {
     detailsButton.style.display = 'none';
     document.getElementById('help-box-outer').style.display = 'block';
   }
+</if>

[afakhry, 2015/01/22 22:27:23]

To mimic the behavior of the old ChromeOS's offline error page, we will never
hide the details link when it's ChromeOS.

[afakhry, 2015/01/23 01:35:46]

@edwardjung, @arv: Can you please confirm that this change won't result in a
weird behavior for ChromeOS?

[afakhry, 2015/01/23 01:42:01]

A bit of context: ChromeOS used to use:
- src/chrome/browser/resources/chromeos/neterror.js
- src/chrome/browser/resources/chromeos/offline_net_load.html

Now we are trying to consolidate and use the same error page used on Chrome:
- src/chrome/renderer/resources/neterror.js
- src/chrome/renderer/resources/neterror.html

With the exception that we don't want to hide the 'details' link.

[edwardjung, 2015/01/23 11:18:28]

It makes sense to consolidate. Is src/chrome/renderer/resources/neterror.html
being updated to provide the 'diagnose' button you show on CrOS? 

[edwardjung, 2015/01/23 11:18:28]

As long as there will always be something in the details section to show, this
should be okay. I can't remember but is there a difference between CrOS and
ChromiumOS where there is sometimes no detailed info / diagnose button. If
that's the case, maybe the JS should be changed to check for a diagnose button,
and remove the "not chromeos" if expression.

Maybe something like:

if (loadTimeData.valueExists('summary') &&
    !loadTimeData.getValue('summary').msg &&
   (!loadTimeData.valueExists('diagnose') || !loadTimeData.getValue('diagnose'))
{
…
}

You'll need to check this for non CrOS platforms.

[afakhry, 2015/01/23 17:49:25]

The diagnose button is only specific to ChromeOS. Setting its value is guarded
with an #if defined(OS_CHROMEOS) in LocalizedError::GetStrings(). Adding this
will change the behavior on other platforms. 

The idea is, on ChromeOS, we always want to show the 'details' link and hide the
extra details by default. But we are afraid that this check:

  if (loadTimeData.valueExists('summary') &&
          !loadTimeData.getValue('summary').msg)

is there for a reason, especially that we don't set summary.msg in
LocalizedError::GetStrings() at:

if (error_code == net::ERR_INTERNET_DISCONNECTED ||
      error_code == chrome_common_net::DNS_PROBE_FINISHED_NO_INTERNET) 

That's why we need your opinion.

 
   // Show control buttons.
   if (loadTimeData.valueExists('reloadButton') &&