Commit f9717895 authored by Robert Knight's avatar Robert Knight Committed by Nick Stenning

Tidy up the comments and variable naming relating to Chrome extension install

Having a variable called 'isChrome' and then explaining why
we are trying to work around Edge's spoofing that aims
to deal with exactly this kind of check is silly.

Instead make it more explicit that we are checking for
support for Chrome _extensions_. If Microsoft Edge supports
all the APIs we need in future for that, great, we'll let
them use it.
parent 54d6f8d3
...@@ -15,13 +15,18 @@ function showSupportedInstallers(rootElement) { ...@@ -15,13 +15,18 @@ function showSupportedInstallers(rootElement) {
} }
} }
var canInstallChromeExt = uaDetect.isChrome && !uaDetect.isMobile; // check for Chrome extension support. We also check here for !mobile
var canInstallBookmarklet = !uaDetect.isChrome && !uaDetect.isMobile && // so that the page is correct when testing in Chrome dev tools with
!uaDetect.isMicrosoftEdge; // mobile device simulation enabled
var offerChromeInstall = uaDetect.chromeExtensionsSupported &&
!uaDetect.isMobile;
var offerBookmarkletInstall = !offerChromeInstall &&
!uaDetect.isMobile &&
!uaDetect.isMicrosoftEdge;
showIf('.js-install-chrome', canInstallChromeExt); showIf('.js-install-chrome', offerChromeInstall);
showIf('.js-install-bookmarklet', canInstallBookmarklet); showIf('.js-install-bookmarklet', offerBookmarkletInstall);
showIf('.js-install-any', canInstallChromeExt || canInstallBookmarklet); showIf('.js-install-any', offerChromeInstall || offerBookmarkletInstall);
} }
module.exports = { module.exports = {
......
...@@ -35,19 +35,21 @@ describe('installer page', function () { ...@@ -35,19 +35,21 @@ describe('installer page', function () {
} }
it('shows the chrome extension to desktop Chrome users', function () { it('shows the chrome extension to desktop Chrome users', function () {
var controller = createController({isChrome: true}); var controller = createController({chromeExtensionsSupported: true});
assert.isFalse(isHidden(extensionBtn)); assert.isFalse(isHidden(extensionBtn));
assert.isTrue(isHidden(bookmarkletBtn)); assert.isTrue(isHidden(bookmarkletBtn));
}); });
it('shows the bookmarklet on desktop browsers', function () { it('shows the bookmarklet on desktop browsers', function () {
var controller = createController({isChrome: true, isMobile: false}); var controller = createController({chromeExtensionsSupported: false,
assert.isFalse(isHidden(extensionBtn)); isMobile: false});
assert.isTrue(isHidden(bookmarkletBtn)); assert.isTrue(isHidden(extensionBtn));
assert.isFalse(isHidden(bookmarkletBtn));
}); });
it('shows only the Via link to mobile browsers', function () { it('shows only the Via link to mobile browsers', function () {
var controller = createController({isChrome: true, isMobile: true}); var controller = createController({chromeExtensionsSupported: false,
isMobile: true});
assert.isTrue(isHidden(extensionBtn)); assert.isTrue(isHidden(extensionBtn));
assert.isTrue(isHidden(bookmarkletBtn)); assert.isTrue(isHidden(bookmarkletBtn));
}); });
......
...@@ -2,8 +2,10 @@ module.exports = { ...@@ -2,8 +2,10 @@ module.exports = {
isMobile: navigator.userAgent.match(/\b(Mobile)\b/), isMobile: navigator.userAgent.match(/\b(Mobile)\b/),
isMicrosoftEdge: navigator.userAgent.match(/\bEdge\b/), isMicrosoftEdge: navigator.userAgent.match(/\bEdge\b/),
// we test for the existence of window.chrome here because Microsoft Edge // we test for the existence of window.chrome.webstore
// (and possibly others) serve // here because Microsoft Edge (and possibly others) include
isChrome: typeof window.chrome !== 'undefined' && // "Chrome" in their UA strings for website compatibility reasons
typeof window.chrome.webstore !== 'undefined', // and Edge also provides an empty "window.chrome" object.
chromeExtensionsSupported: typeof window.chrome !== 'undefined' &&
typeof window.chrome.webstore !== 'undefined',
}; };
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment