From 3f7916e8a610ed60a3d03469453c778499fddc77 Mon Sep 17 00:00:00 2001 From: Krystle Salazar Date: Wed, 3 Jun 2020 08:36:35 +0000 Subject: [PATCH] Bug 1570678 - Replace (i) icon for a file icon on potentially trustworthy pages. r=johannh,nhnt11 Differential Revision: https://phabricator.services.mozilla.com/D70932 --- browser/base/content/browser-siteIdentity.js | 33 ++++++++++--- .../browser_check_identity_state.js | 30 +++++++----- .../browser_deprecatedTLSVersions.js | 47 +++++++++++-------- .../browser_identityBlock_flicker.js | 2 +- .../browser_identityIcon_img_url.js | 13 +++-- ...rowser_secure_transport_insecure_scheme.js | 2 +- .../themes/shared/controlcenter/panel.inc.css | 4 ++ .../identity-block/identity-block.inc.css | 4 ++ toolkit/themes/shared/icons/document.svg | 6 +++ toolkit/themes/shared/jar.inc.mn | 1 + 10 files changed, 96 insertions(+), 46 deletions(-) create mode 100644 toolkit/themes/shared/icons/document.svg diff --git a/browser/base/content/browser-siteIdentity.js b/browser/base/content/browser-siteIdentity.js index 077c78e870b3..ff2afc870ee0 100644 --- a/browser/base/content/browser-siteIdentity.js +++ b/browser/base/content/browser-siteIdentity.js @@ -131,6 +131,23 @@ var gIdentityHandler = { ); }, + get _isAboutNetErrorPage() { + return ( + gBrowser.selectedBrowser.documentURI && + gBrowser.selectedBrowser.documentURI.scheme == "about" && + gBrowser.selectedBrowser.documentURI.pathQueryRef.startsWith("neterror") + ); + }, + + get _isPotentiallyTrustworthy() { + return ( + !this._isBrokenConnection && + (this._isSecureContext || + (gBrowser.selectedBrowser.documentURI && + gBrowser.selectedBrowser.documentURI.scheme == "chrome")) + ); + }, + // smart getters get _identityPopup() { delete this._identityPopup; @@ -707,14 +724,12 @@ var gIdentityHandler = { } else if (this._isAboutCertErrorPage) { // We show a warning lock icon for 'about:certerror' page. this._identityBox.className = "certErrorPage"; - } else if ( - this._isSecureContext || - (gBrowser.selectedBrowser.documentURI && - (gBrowser.selectedBrowser.documentURI.scheme == "about" || - gBrowser.selectedBrowser.documentURI.scheme == "chrome")) - ) { - // This is a local resource (and shouldn't be marked insecure). + } else if (this._isAboutNetErrorPage) { + // Network errors get a more neutral icon this._identityBox.className = "unknownIdentity"; + } else if (this._isPotentiallyTrustworthy) { + // This is a local resource (and shouldn't be marked insecure). + this._identityBox.className = "localResource"; } else { // This is an insecure connection. let warnOnInsecure = @@ -893,6 +908,10 @@ var gIdentityHandler = { customRoot = this._hasCustomRoot(); } else if (this._isAboutCertErrorPage) { connection = "cert-error-page"; + } else if (this._isAboutNetErrorPage) { + connection = "not-secure"; + } else if (this._isPotentiallyTrustworthy) { + connection = "file"; } // Determine the mixed content state. diff --git a/browser/base/content/test/siteIdentity/browser_check_identity_state.js b/browser/base/content/test/siteIdentity/browser_check_identity_state.js index 5ec7bca98316..82d9191d29cb 100644 --- a/browser/base/content/test/siteIdentity/browser_check_identity_state.js +++ b/browser/base/content/test/siteIdentity/browser_check_identity_state.js @@ -61,7 +61,7 @@ async function webpageTest(secureCheck) { } gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; if (secureCheck) { @@ -96,7 +96,7 @@ async function webpageTestTextWarning(secureCheck) { } gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; if (secureCheck) { @@ -180,7 +180,7 @@ async function blankPageTest(secureCheck) { "valid", "pageproxystate should be valid" ); - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; is( @@ -207,7 +207,7 @@ async function secureTest(secureCheck) { is(getIdentityMode(), "verifiedDomain", "Identity should be verified"); gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; is(getIdentityMode(), "verifiedDomain", "Identity should be verified"); @@ -257,7 +257,7 @@ async function insecureTest(secureCheck) { } gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; if (secureCheck) { @@ -290,7 +290,7 @@ async function addonsTest(secureCheck) { is(getIdentityMode(), "chromeUI", "Identity should be chrome"); gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; is(getIdentityMode(), "chromeUI", "Identity should be chrome"); @@ -315,7 +315,7 @@ async function fileTest(secureCheck) { is(getConnectionState(), "file", "Connection should be file"); gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; is(getConnectionState(), "file", "Connection should be file"); @@ -341,7 +341,11 @@ async function resourceUriTest(secureCheck) { is(getConnectionState(), "file", "Connection should be file"); gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is( + getIdentityMode(), + "localResource", + "Identity should be a local a resource" + ); gBrowser.selectedTab = newTab; is(getConnectionState(), "file", "Connection should be file"); @@ -378,7 +382,7 @@ async function noCertErrorTest(secureCheck) { ); gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; is( @@ -471,7 +475,7 @@ async function aboutUriTest(secureCheck) { is(getConnectionState(), "file", "Connection should be file"); gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; is(getConnectionState(), "file", "Connection should be file"); @@ -522,7 +526,7 @@ async function dataUriTest(secureCheck) { } gBrowser.selectedTab = oldTab; - is(getIdentityMode(), "unknownIdentity", "Identity should be unknown"); + is(getIdentityMode(), "localResource", "Identity should be localResource"); gBrowser.selectedTab = newTab; if (secureCheck) { @@ -574,8 +578,8 @@ async function pbModeTest(prefs, secureCheck) { privateWin.gBrowser.selectedTab = oldTab; is( getIdentityMode(privateWin), - "unknownIdentity", - "Identity should be unknown" + "localResource", + "Identity should be localResource" ); privateWin.gBrowser.selectedTab = newTab; diff --git a/browser/base/content/test/siteIdentity/browser_deprecatedTLSVersions.js b/browser/base/content/test/siteIdentity/browser_deprecatedTLSVersions.js index 743bd160ad66..7f7261997492 100644 --- a/browser/base/content/test/siteIdentity/browser_deprecatedTLSVersions.js +++ b/browser/base/content/test/siteIdentity/browser_deprecatedTLSVersions.js @@ -14,10 +14,29 @@ function getIdentityMode(aWindow = window) { return aWindow.document.getElementById("identity-box").className; } +function openIdentityPopup() { + let mainView = document.getElementById("identity-popup-mainView"); + let viewShown = BrowserTestUtils.waitForEvent(mainView, "ViewShown"); + gIdentityHandler._identityBox.click(); + return viewShown; +} + +function closeIdentityPopup() { + let promise = BrowserTestUtils.waitForEvent( + gIdentityHandler._identityPopup, + "popuphidden" + ); + gIdentityHandler._identityPopup.hidePopup(); + return promise; +} + +async function checkConnectionState(state) { + await openIdentityPopup(); + is(getConnectionState(), state, "connectionState should be " + state); + await closeIdentityPopup(); +} + function getConnectionState() { - // Prevents items that are being lazy loaded causing issues - document.getElementById("identity-box").click(); - gIdentityHandler.refreshIdentityPopup(); return document.getElementById("identity-popup").getAttribute("connection"); } @@ -42,11 +61,7 @@ add_task(async function() { "unknownIdentity weakCipher", "Identity should be unknownIdentity" ); - is( - getConnectionState(), - "not-secure", - "connectionState should be not-secure" - ); + await checkConnectionState("not-secure"); await BrowserTestUtils.loadURI(browser, HTTPS_TLS1_1); await BrowserTestUtils.browserLoaded(browser); @@ -56,18 +71,14 @@ add_task(async function() { "unknownIdentity weakCipher", "Identity should be unknownIdentity" ); - is( - getConnectionState(), - "not-secure", - "connectionState should be not-secure" - ); + await checkConnectionState("not-secure"); // Transition to secure await BrowserTestUtils.loadURI(browser, HTTPS_TLS1_2); await BrowserTestUtils.browserLoaded(browser); isSecurityState(browser, "secure"); is(getIdentityMode(), "verifiedDomain", "Identity should be verified"); - is(getConnectionState(), "secure", "connectionState should be secure"); + await checkConnectionState("secure"); // Transition back to broken await BrowserTestUtils.loadURI(browser, HTTPS_TLS1_1); @@ -78,17 +89,13 @@ add_task(async function() { "unknownIdentity weakCipher", "Identity should be unknownIdentity" ); - is( - getConnectionState(), - "not-secure", - "connectionState should be not-secure" - ); + await checkConnectionState("not-secure"); // TLS1.3 for completeness await BrowserTestUtils.loadURI(browser, HTTPS_TLS1_3); await BrowserTestUtils.browserLoaded(browser); isSecurityState(browser, "secure"); is(getIdentityMode(), "verifiedDomain", "Identity should be verified"); - is(getConnectionState(), "secure", "connectionState should be secure"); + await checkConnectionState("secure"); }); }); diff --git a/browser/base/content/test/siteIdentity/browser_identityBlock_flicker.js b/browser/base/content/test/siteIdentity/browser_identityBlock_flicker.js index e71d2ff2a3cd..1e8d24943a0c 100644 --- a/browser/base/content/test/siteIdentity/browser_identityBlock_flicker.js +++ b/browser/base/content/test/siteIdentity/browser_identityBlock_flicker.js @@ -14,7 +14,7 @@ add_task(async function test() { is( identityBox.className, - "unknownIdentity", + "localResource", "identity box has the correct class" ); diff --git a/browser/base/content/test/siteIdentity/browser_identityIcon_img_url.js b/browser/base/content/test/siteIdentity/browser_identityIcon_img_url.js index abd3acec97e1..f2e531d19881 100644 --- a/browser/base/content/test/siteIdentity/browser_identityIcon_img_url.js +++ b/browser/base/content/test/siteIdentity/browser_identityIcon_img_url.js @@ -30,7 +30,7 @@ const TEST_CASES = [ { type: "non-chrome about page", testURL: "about:about", - img_url: `url("chrome://global/skin/icons/identity-icon.svg")`, + img_url: `url("chrome://global/skin/icons/document.svg")`, }, { type: "chrome about page", @@ -40,7 +40,12 @@ const TEST_CASES = [ { type: "file", testURL: "dummy_page.html", - img_url: `url("chrome://global/skin/icons/identity-icon.svg")`, + img_url: `url("chrome://global/skin/icons/document.svg")`, + }, + { + type: "resource", + testURL: "resource://gre/modules/Log.jsm", + img_url: `url("chrome://global/skin/icons/document.svg")`, }, { type: "mixedPassiveContent", @@ -60,12 +65,12 @@ const TEST_CASES = [ { type: "localhost", testURL: "http://127.0.0.1", - img_url: `url("chrome://global/skin/icons/identity-icon.svg")`, + img_url: `url("chrome://global/skin/icons/document.svg")`, }, { type: "localhost + http frame", testURL: kBaseURILocalhost + "file_csp_block_all_mixedcontent.html", - img_url: `url("chrome://global/skin/icons/identity-icon.svg")`, + img_url: `url("chrome://global/skin/icons/document.svg")`, }, { type: "data URI", diff --git a/browser/base/content/test/siteIdentity/browser_secure_transport_insecure_scheme.js b/browser/base/content/test/siteIdentity/browser_secure_transport_insecure_scheme.js index 22ffa817eecb..ac6620e10c68 100644 --- a/browser/base/content/test/siteIdentity/browser_secure_transport_insecure_scheme.js +++ b/browser/base/content/test/siteIdentity/browser_secure_transport_insecure_scheme.js @@ -26,7 +26,7 @@ add_task(async function() { }); await BrowserTestUtils.browserLoaded(browser); let identityMode = window.document.getElementById("identity-box").className; - is(identityMode, "unknownIdentity", "identity should be 'unknown'"); + is(identityMode, "localResource", "identity should be 'localResource'"); }); }); diff --git a/browser/themes/shared/controlcenter/panel.inc.css b/browser/themes/shared/controlcenter/panel.inc.css index 6a0bef3fe230..d12d1081559f 100644 --- a/browser/themes/shared/controlcenter/panel.inc.css +++ b/browser/themes/shared/controlcenter/panel.inc.css @@ -369,6 +369,10 @@ background-image: url(chrome://branding/content/icon48.png); } +#identity-popup[connection=file] .identity-popup-security-connection { + background-image: url(chrome://global/skin/icons/document.svg); +} + #identity-popup[connection^=secure] .identity-popup-security-connection { background-image: url(chrome://browser/skin/connection-secure.svg); -moz-context-properties: fill; diff --git a/browser/themes/shared/identity-block/identity-block.inc.css b/browser/themes/shared/identity-block/identity-block.inc.css index e358a3e23f82..a5a48657c01c 100644 --- a/browser/themes/shared/identity-block/identity-block.inc.css +++ b/browser/themes/shared/identity-block/identity-block.inc.css @@ -124,6 +124,10 @@ list-style-image: url(chrome://branding/content/identity-icons-brand.svg); } +#identity-box[pageproxystate="valid"].localResource > #identity-icon { + list-style-image: url(chrome://global/skin/icons/document.svg); +} + #urlbar:not(.searchButton) > #urlbar-input-container > #identity-box[pageproxystate="invalid"] > #identity-icon { list-style-image: url(chrome://browser/skin/search-glass.svg); fill-opacity: .4; diff --git a/toolkit/themes/shared/icons/document.svg b/toolkit/themes/shared/icons/document.svg new file mode 100644 index 000000000000..4f43653a0e85 --- /dev/null +++ b/toolkit/themes/shared/icons/document.svg @@ -0,0 +1,6 @@ + + + + diff --git a/toolkit/themes/shared/jar.inc.mn b/toolkit/themes/shared/jar.inc.mn index a05a03931b97..6ee4caa77bf7 100644 --- a/toolkit/themes/shared/jar.inc.mn +++ b/toolkit/themes/shared/jar.inc.mn @@ -40,6 +40,7 @@ toolkit.jar: skin/classic/global/pictureinpicture/close-pip.svg (../../shared/pictureinpicture/close-pip.svg) skin/classic/global/icons/columnpicker.svg (../../shared/icons/columnpicker.svg) skin/classic/global/icons/delete.svg (../../shared/icons/delete.svg) + skin/classic/global/icons/document.svg (../../shared/icons/document.svg) skin/classic/global/icons/error.svg (../../shared/icons/error.svg) skin/classic/global/icons/find-previous-arrow.svg (../../shared/icons/find-previous-arrow.svg) skin/classic/global/icons/find-next-arrow.svg (../../shared/icons/find-next-arrow.svg)