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
This commit is contained in:
jdlrobson 2019-07-31 16:17:20 -07:00 committed by Jdlrobson
parent 02d5a878a1
commit a0e5139e40
3 changed files with 18 additions and 6 deletions

View File

@ -4,7 +4,7 @@
ToggleList = require( '../../components/ToggleList/ToggleList.js' ), ToggleList = require( '../../components/ToggleList/ToggleList.js' ),
downloadPageAction = require( './downloadPageAction.js' ).downloadPageAction, downloadPageAction = require( './downloadPageAction.js' ).downloadPageAction,
Icon = mobile.Icon, Icon = mobile.Icon,
skin = mobile.Skin.getSingleton(), page = mobile.currentPage(),
/** The top level menu. */ /** The top level menu. */
toolbarSelector = '.page-actions-menu', toolbarSelector = '.page-actions-menu',
/** The secondary overflow submenu component container. */ /** The secondary overflow submenu component container. */
@ -83,7 +83,7 @@
* @return {void} * @return {void}
*/ */
function renderDownloadButton( window, overflowList ) { function renderDownloadButton( window, overflowList ) {
var $downloadAction = downloadPageAction( skin, var $downloadAction = downloadPageAction( page,
mw.config.get( 'wgMinervaDownloadNamespaces', [] ), window, !!overflowList ); mw.config.get( 'wgMinervaDownloadNamespaces', [] ), window, !!overflowList );
if ( $downloadAction ) { if ( $downloadAction ) {

View File

@ -45,9 +45,10 @@
chromeVersion = getChromeVersion( userAgent ); chromeVersion = getChromeVersion( userAgent );
// Download button is restricted to certain namespaces T181152. // Download button is restricted to certain namespaces T181152.
// Not shown on missing pages
// Defaults to 0, in case cached JS has been served. // Defaults to 0, in case cached JS has been served.
if ( supportedNamespaces.indexOf( page.getNamespaceId() ) === -1 || if ( supportedNamespaces.indexOf( page.getNamespaceId() ) === -1 ||
page.isMainPage() ) { page.isMainPage() || page.isMissing ) {
// namespace is not supported or it's a main page // namespace is not supported or it's a main page
return false; return false;
} }
@ -117,13 +118,13 @@
* Generate a download icon for triggering print functionality if * Generate a download icon for triggering print functionality if
* printing is available * printing is available
* *
* @param {Skin} skin * @param {Page} page
* @param {number[]} supportedNamespaces * @param {number[]} supportedNamespaces
* @param {Window} [windowObj] window object * @param {Window} [windowObj] window object
* @param {boolean} [hasText] Use icon + button style. * @param {boolean} [hasText] Use icon + button style.
* @returns {jQuery.Object|null} * @returns {jQuery.Object|null}
*/ */
function downloadPageAction( skin, supportedNamespaces, windowObj, hasText ) { function downloadPageAction( page, supportedNamespaces, windowObj, hasText ) {
var var
modifier = hasText ? 'toggle-list-item__anchor--menu' : 'mw-ui-icon-element', modifier = hasText ? 'toggle-list-item__anchor--menu' : 'mw-ui-icon-element',
icon, icon,
@ -134,7 +135,7 @@
if ( if (
isAvailable( isAvailable(
windowObj, skin.page, navigator.userAgent, windowObj, page, navigator.userAgent,
supportedNamespaces supportedNamespaces
) )
) { ) {

View File

@ -86,6 +86,7 @@
var page = new Page( { var page = new Page( {
id: 0, id: 0,
title: 'Test', title: 'Test',
isMissing: false,
isMainPage: false isMainPage: false
} ); } );
this.page = page; this.page = page;
@ -108,10 +109,20 @@
assert.notOk( isAvailable( windowChrome, this.page, VALID_UA, [ 9999 ] ) ); 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 ) { QUnit.test( 'isAvailable() handles properly main page', function ( assert ) {
var page = new Page( { var page = new Page( {
id: 0, id: 0,
title: 'Test', title: 'Test',
isMissing: false,
isMainPage: true isMainPage: true
} ); } );
assert.notOk( isAvailable( windowChrome, page, VALID_UA, VALID_SUPPORTED_NAMESPACES ) ); assert.notOk( isAvailable( windowChrome, page, VALID_UA, VALID_SUPPORTED_NAMESPACES ) );