From 54d25a7e24878938fe50bc65fb336e605a4e72ac Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Mon, 22 Apr 2013 11:07:08 +0200 Subject: [PATCH] mediawiki.util: Fix roundtripping of tooltip in portlet links In 558985f72a ctrl-option- for Chrome on Mac was added, but it didn't add "option-" to the regex. Since then the tooltip in Chrome on Mac (and later when we started referring to option instead of alt on all Mac browsers) was never detected by this regex, sometimes resulting in double tooltips (when adding) or outdated tooltips (when updating). Now we always look for an accesskey hint in the tooltip and strip it it's there. Also fixed a bug where an undefined error can occur if accesskey is given but tooltip not (the method unconditinally appended text to the tooltip varible which might be undefined causing a tooltip like "undefined[a]"). Change-Id: I0bde1a228983c58b20cad0c09a8e5efe8225ea23 --- RELEASE-NOTES-1.22 | 8 +++ resources/mediawiki/mediawiki.util.js | 35 +++++++++---- .../mediawiki/mediawiki.util.test.js | 49 +++++++++++++++---- 3 files changed, 74 insertions(+), 18 deletions(-) diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22 index cf31e5132a..d3ea77e56f 100644 --- a/RELEASE-NOTES-1.22 +++ b/RELEASE-NOTES-1.22 @@ -35,6 +35,10 @@ production. Pager-related WebRequest parameters by default, as this is overwhelmingly likely to be what was intended by users of the method. If any caller wishes to use these parameters, the new param 'useRequestParams' may be set to true. +* mw.util.addPortletLink: Tooltip is no longer required to be plain (without + an accesskey in it already). As such it now rountrips. Creating a link with a + message as tooltip, grabbing the title attribute and using it to create + another portlet will work as expected. === Bug fixes in 1.22 === * Disable Special:PasswordReset when $wgEnableEmail. Previously one could still @@ -48,6 +52,8 @@ production. * (bug 47218) Special:BlockList now handles correctly user names with spaces when passed as subpage. * Pager's properly validate which fields are allowed to be sorted on. +* mw.util.tooltipAccessKeyRegexp: The regex now matches "option-" as well. + Support for Mac "option" was added in 1.16, but the regex was never updated. === API changes in 1.22 === * (bug 46626) xmldoublequote parameter was removed. Because of a bug, the @@ -85,6 +91,8 @@ changes to languages because of Bugzilla reports. * Calling Linker methods using a skin will now output deprecation warnings. * (bug 46680) "Return to" links are no longer tagged with rel="next". * The Special:ActiveUsers special page was removed. +* BREAKING CHANGE: mw.util.tooltipAccessKeyRegexp: The match group for the + accesskey character is now $6 instead of $5. == Compatibility == diff --git a/resources/mediawiki/mediawiki.util.js b/resources/mediawiki/mediawiki.util.js index 1bd7430cca..60ef7584a2 100644 --- a/resources/mediawiki/mediawiki.util.js +++ b/resources/mediawiki/mediawiki.util.js @@ -279,8 +279,17 @@ /** * @property {RegExp} * Regex to match accesskey tooltips. + * + * Should match: + * + * - "ctrl-option-" + * - "alt-shift-" + * - "ctrl-alt-" + * - "ctrl-" + * + * The accesskey is matched in group $6. */ - tooltipAccessKeyRegexp: /\[(ctrl-)?(alt-)?(shift-)?(esc-)?(.)\]$/, + tooltipAccessKeyRegexp: /\[(ctrl-)?(option-)?(alt-)?(shift-)?(esc-)?(.)\]$/, /** * Add the appropriate prefix to the accesskey shown in the tooltip. @@ -301,9 +310,9 @@ } $nodes.attr( 'title', function ( i, val ) { - if ( val && util.tooltipAccessKeyRegexp.exec( val ) ) { + if ( val && util.tooltipAccessKeyRegexp.test( val ) ) { return val.replace( util.tooltipAccessKeyRegexp, - '[' + util.tooltipAccessKeyPrefix + '$5]' ); + '[' + util.tooltipAccessKeyPrefix + '$6]' ); } return val; } ); @@ -406,19 +415,27 @@ if ( id ) { $item.attr( 'id', id ); } - if ( accesskey ) { - $link.attr( 'accesskey', accesskey ); - tooltip += ' [' + accesskey + ']'; + + if ( tooltip ) { + // Trim any existing accesskey hint and the trailing space + tooltip = $.trim( tooltip.replace( util.tooltipAccessKeyRegexp, '' ) ); + if ( accesskey ) { + tooltip += ' [' + accesskey + ']'; + } $link.attr( 'title', tooltip ); + if ( accesskey ) { + util.updateTooltipAccessKeys( $link ); + } } - if ( accesskey && tooltip ) { - util.updateTooltipAccessKeys( $link ); + + if ( accesskey ) { + $link.attr( 'accesskey', accesskey ); } // Where to put our node ? // - nextnode is a DOM element (was the only option before MW 1.17, in wikibits.js) if ( nextnode && nextnode.parentNode === $ul[0] ) { - $(nextnode).before( $item ); + $( nextnode ).before( $item ); // - nextnode is a CSS selector for jQuery } else if ( typeof nextnode === 'string' && $ul.find( nextnode ).length !== 0 ) { diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js index bba3160a11..713ec4be5b 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js @@ -1,5 +1,13 @@ ( function ( mw, $ ) { - QUnit.module( 'mediawiki.util', QUnit.newMwEnvironment() ); + QUnit.module( 'mediawiki.util', QUnit.newMwEnvironment( { + setup: function () { + this.taPrefix = mw.util.tooltipAccessKeyPrefix; + mw.util.tooltipAccessKeyPrefix = 'ctrl-alt-'; + }, + teardown: function () { + mw.util.tooltipAccessKeyPrefix = this.taPrefix; + } + } ) ); QUnit.test( 'rawurlencode', 1, function ( assert ) { assert.equal( mw.util.rawurlencode( 'Test:A & B/Here' ), 'Test%3AA%20%26%20B%2FHere' ); @@ -108,10 +116,14 @@ assert.strictEqual( mw.util.getParamValue( 'TEST', url ), 'a b+c d', 'Bug 30441: getParamValue must understand "+" encoding of space (multiple spaces)' ); } ); - QUnit.test( 'tooltipAccessKey', 3, function ( assert ) { - assert.equal( typeof mw.util.tooltipAccessKeyPrefix, 'string', 'mw.util.tooltipAccessKeyPrefix must be a string' ); - assert.ok( mw.util.tooltipAccessKeyRegexp instanceof RegExp, 'mw.util.tooltipAccessKeyRegexp instance of RegExp' ); - assert.ok( mw.util.updateTooltipAccessKeys, 'mw.util.updateTooltipAccessKeys' ); + QUnit.test( 'tooltipAccessKey', 4, function ( assert ) { + assert.equal( typeof mw.util.tooltipAccessKeyPrefix, 'string', 'tooltipAccessKeyPrefix must be a string' ); + assert.equal( $.type( mw.util.tooltipAccessKeyRegexp ), 'regexp', 'tooltipAccessKeyRegexp is a regexp' ); + assert.ok( mw.util.updateTooltipAccessKeys, 'updateTooltipAccessKeys is non-empty' ); + + 'Example [a]'.replace( mw.util.tooltipAccessKeyRegexp, function ( sub, m1, m2, m3, m4, m5, m6 ) { + assert.equal( m6, 'a', 'tooltipAccessKeyRegexp finds the accesskey hint' ); + } ); } ); QUnit.test( '$content', 2, function ( assert ) { @@ -125,7 +137,7 @@ * Previously, test elements where invisible to the selector since only * one element can have a given id. */ - QUnit.test( 'addPortletLink', 8, function ( assert ) { + QUnit.test( 'addPortletLink', 10, function ( assert ) { var pTestTb, pCustom, vectorTabs, tbRL, cuQuux, $cuQuux, tbMW, $tbMW, tbRLDM, caFoo; pTestTb = '\ @@ -154,7 +166,8 @@ $( '#qunit-fixture' ).append( pTestTb, pCustom, vectorTabs ); tbRL = mw.util.addPortletLink( 'p-test-tb', '//mediawiki.org/wiki/ResourceLoader', - 'ResourceLoader', 't-rl', 'More info about ResourceLoader on MediaWiki.org ', 'l' ); + 'ResourceLoader', 't-rl', 'More info about ResourceLoader on MediaWiki.org ', 'l' + ); assert.ok( $.isDomElement( tbRL ), 'addPortletLink returns a valid DOM Element according to $.isDomElement' ); @@ -162,14 +175,32 @@ 'MediaWiki.org', 't-mworg', 'Go to MediaWiki.org ', 'm', tbRL ); $tbMW = $( tbMW ); + assert.propEqual( + $tbMW.getAttrs(), + { + id: 't-mworg' + }, + 'Validate attributes of created element' + ); + + assert.propEqual( + $tbMW.find( 'a' ).getAttrs(), + { + href: '//mediawiki.org/', + title: 'Go to MediaWiki.org [ctrl-alt-m]', + accesskey: 'm' + }, + 'Validate attributes of anchor tag in created element' + ); - assert.equal( $tbMW.attr( 'id' ), 't-mworg', 'Link has correct ID set' ); assert.equal( $tbMW.closest( '.portlet' ).attr( 'id' ), 'p-test-tb', 'Link was inserted within correct portlet' ); assert.equal( $tbMW.next().attr( 'id' ), 't-rl', 'Link is in the correct position (by passing nextnode)' ); - cuQuux = mw.util.addPortletLink( 'p-test-custom', '#', 'Quux' ); + cuQuux = mw.util.addPortletLink( 'p-test-custom', '#', 'Quux', null, 'Example [shift-x]', 'q' ); $cuQuux = $( cuQuux ); + assert.equal( $cuQuux.find( 'a' ).attr( 'title' ), 'Example [ctrl-alt-q]', 'Existing accesskey is stripped and updated' ); + assert.equal( $( '#p-test-custom #c-barmenu ul li' ).length, 1, -- 2.20.1