Krinkle has uploaded a new change for review.
https://gerrit.wikimedia.org/r/60247
Change subject: mediawiki.util: Fix roundtripping of tooltip in portlet links
......................................................................
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
---
M RELEASE-NOTES-1.22
M resources/mediawiki/mediawiki.util.js
M tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js
3 files changed, 74 insertions(+), 18 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/47/60247/1
diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 13a3c3c..e42cc7d 100644
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -31,6 +31,10 @@
by adding a new configuration variable $wgApplyIpBlocksToXff (disabled by
default).
* The new hook 'APIGetPossibleErrors' to modify the list of possible errors was
added.
+* 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
@@ -44,6 +48,8 @@
* (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
@@ -81,6 +87,8 @@
* 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 1bd7430..60ef758 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 ( 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 ) {
$link.attr( 'accesskey', accesskey );
- tooltip += ' [' + accesskey + ']';
- $link.attr( 'title', tooltip );
- }
- if ( accesskey && tooltip ) {
- util.updateTooltipAccessKeys( $link );
}
// 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 bba3160..713ec4b 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.equal( $tbMW.attr( 'id' ), 't-mworg', 'Link has correct
ID set' );
+ 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.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,
--
To view, visit https://gerrit.wikimedia.org/r/60247
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0bde1a228983c58b20cad0c09a8e5efe8225ea23
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <krinklem...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
