From a0e5139e4006fb5b4eceb83349ca5ad39c64c43a Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Wed, 31 Jul 2019 16:17:20 -0700 Subject: [PATCH] Don't show download button on missing pages Additional: Pass in a page rather than a skin - skin is not used anywhere other than to get the page Bug: T211775 Change-Id: Ia7c56158773ac16992fb1ebf002131e9c24dda14 --- resources/skins.minerva.scripts/Toolbar.js | 4 ++-- resources/skins.minerva.scripts/downloadPageAction.js | 9 +++++---- .../skins.minerva.scripts/downloadPageAction.test.js | 11 +++++++++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/resources/skins.minerva.scripts/Toolbar.js b/resources/skins.minerva.scripts/Toolbar.js index 429bdb5..eb16f80 100644 --- a/resources/skins.minerva.scripts/Toolbar.js +++ b/resources/skins.minerva.scripts/Toolbar.js @@ -4,7 +4,7 @@ ToggleList = require( '../../components/ToggleList/ToggleList.js' ), downloadPageAction = require( './downloadPageAction.js' ).downloadPageAction, Icon = mobile.Icon, - skin = mobile.Skin.getSingleton(), + page = mobile.currentPage(), /** The top level menu. */ toolbarSelector = '.page-actions-menu', /** The secondary overflow submenu component container. */ @@ -83,7 +83,7 @@ * @return {void} */ function renderDownloadButton( window, overflowList ) { - var $downloadAction = downloadPageAction( skin, + var $downloadAction = downloadPageAction( page, mw.config.get( 'wgMinervaDownloadNamespaces', [] ), window, !!overflowList ); if ( $downloadAction ) { diff --git a/resources/skins.minerva.scripts/downloadPageAction.js b/resources/skins.minerva.scripts/downloadPageAction.js index df3e689..a3daa4e 100644 --- a/resources/skins.minerva.scripts/downloadPageAction.js +++ b/resources/skins.minerva.scripts/downloadPageAction.js @@ -45,9 +45,10 @@ chromeVersion = getChromeVersion( userAgent ); // Download button is restricted to certain namespaces T181152. + // Not shown on missing pages // Defaults to 0, in case cached JS has been served. if ( supportedNamespaces.indexOf( page.getNamespaceId() ) === -1 || - page.isMainPage() ) { + page.isMainPage() || page.isMissing ) { // namespace is not supported or it's a main page return false; } @@ -117,13 +118,13 @@ * Generate a download icon for triggering print functionality if * printing is available * - * @param {Skin} skin + * @param {Page} page * @param {number[]} supportedNamespaces * @param {Window} [windowObj] window object * @param {boolean} [hasText] Use icon + button style. * @returns {jQuery.Object|null} */ - function downloadPageAction( skin, supportedNamespaces, windowObj, hasText ) { + function downloadPageAction( page, supportedNamespaces, windowObj, hasText ) { var modifier = hasText ? 'toggle-list-item__anchor--menu' : 'mw-ui-icon-element', icon, @@ -134,7 +135,7 @@ if ( isAvailable( - windowObj, skin.page, navigator.userAgent, + windowObj, page, navigator.userAgent, supportedNamespaces ) ) { diff --git a/tests/qunit/skins.minerva.scripts/downloadPageAction.test.js b/tests/qunit/skins.minerva.scripts/downloadPageAction.test.js index 0698be3..c3fbc2f 100644 --- a/tests/qunit/skins.minerva.scripts/downloadPageAction.test.js +++ b/tests/qunit/skins.minerva.scripts/downloadPageAction.test.js @@ -86,6 +86,7 @@ var page = new Page( { id: 0, title: 'Test', + isMissing: false, isMainPage: false } ); this.page = page; @@ -108,10 +109,20 @@ assert.notOk( isAvailable( windowChrome, this.page, VALID_UA, [ 9999 ] ) ); } ); + QUnit.test( 'isAvailable() handles missing pages', function ( assert ) { + var page = new Page( { + id: 0, + title: 'Missing', + isMissing: true + } ); + assert.notOk( isAvailable( windowChrome, page, VALID_UA, VALID_SUPPORTED_NAMESPACES ) ); + } ); + QUnit.test( 'isAvailable() handles properly main page', function ( assert ) { var page = new Page( { id: 0, title: 'Test', + isMissing: false, isMainPage: true } ); assert.notOk( isAvailable( windowChrome, page, VALID_UA, VALID_SUPPORTED_NAMESPACES ) );