mw.config: Show deprecation notices when accessing globals

The mw.config.value object is no longer an alias to the global object
when $wgLegacyJavaScriptGlobals is true.

Instead, set() is made to copy to property to the global object. This
matches behaviour of other deprecated properties in that changes to the
deprecated property directly are ignored.

Bug: T58550
Change-Id: I703f7c12b59bc3207b2a291eacc393a8ae92df6f

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index 94b64b9..6bf93f4 100644 (file)
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -79,12 +79,56 @@
         * @class mw.Map
         *
         * @constructor
-        * @param {Object|boolean} [values] Value-bearing object to map, or boolean
-        *  true to map over the global object. Defaults to an empty object.
+        * @param {Object|boolean} [values] Value-bearing object to map, defaults to an empty object.
+        *  For backwards-compatibility with mw.config, this can also be `true` in which case values
+        *  will be copied to the Window object as global variables (T72470). Values are copied in one
+        *  direction only. Changes to globals are not reflected in the map.
         */
        function Map( values ) {
-               this.values = values === true ? window : ( values || {} );
-               return this;
+               if ( values === true ) {
+                       this.values = {};
+
+                       // Override #set to also set the global variable
+                       this.set = function ( selection, value ) {
+                               var s;
+
+                               if ( $.isPlainObject( selection ) ) {
+                                       for ( s in selection ) {
+                                               setGlobalMapValue( this, s, selection[s] );
+                                       }
+                                       return true;
+                               }
+                               if ( typeof selection === 'string' && arguments.length ) {
+                                       setGlobalMapValue( this, selection, value );
+                                       return true;
+                               }
+                               return false;
+                       };
+
+                       return;
+               }
+
+               this.values = values || {};
+       }
+
+       /**
+        * Alias property to the global object.
+        *
+        * @private
+        * @static
+        * @param {mw.Map} map
+        * @param {string} key
+        * @param {Mixed} value
+        */
+       function setGlobalMapValue( map, key, value ) {
+               map.values[key] = value;
+               mw.log.deprecate(
+                       window,
+                       key,
+                       value,
+                       // Deprecation notice for mw.config globals (T58550, T72470)
+                       map === mw.config && 'Use mw.config instead.'
+               );
        }
 
        Map.prototype = {
@@ -136,7 +180,7 @@
                 *
                 * @param {string|Object} selection String key to set value for, or object mapping keys to values.
                 * @param {Mixed} [value] Value to set (optional, only in use when key is a string)
-                * @return {Boolean} This returns true on success, false on failure.
+                * @return {boolean} This returns true on success, false on failure.
                 */
                set: function ( selection, value ) {
                        var s;
@@ -147,7 +191,7 @@
                                }
                                return true;
                        }
-                       if ( typeof selection === 'string' && arguments.length > 1 ) {
+                       if ( typeof selection === 'string' && arguments.length ) {
                                this.values[selection] = value;
                                return true;
                        }
@@ -582,6 +626,7 @@
                                        } );
                                } catch ( err ) {
                                        // IE8 can throw on Object.defineProperty
+                                       // Create a copy of the value to the object.
                                        obj[key] = val;
                                }
                        };

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
index 87520bd..6c8c62f 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
@@ -55,7 +55,7 @@
                this.restoreWarnings();
        } );
 
-       QUnit.test( 'mw.Map', 28, function ( assert ) {
+       QUnit.test( 'mw.Map', 34, function ( assert ) {
                var arry, conf, funky, globalConf, nummy, someValues;
 
                conf = new mw.Map();
@@ -126,12 +126,31 @@
 
                conf.set( 'globalMapChecker', 'Hi' );
 
-               assert.ok( 'globalMapChecker' in window === false, 'new mw.Map did not store its values in the global window object by default' );
+               assert.ok( ( 'globalMapChecker' in window ) === false, 'Map does not its store values in the window object by default' );
 
                globalConf = new mw.Map( true );
                globalConf.set( 'anotherGlobalMapChecker', 'Hello' );
 
-               assert.ok( 'anotherGlobalMapChecker' in window, 'new mw.Map( true ) did store its values in the global window object' );
+               assert.ok( 'anotherGlobalMapChecker' in window, 'global Map stores its values in the window object' );
+
+               assert.equal( globalConf.get( 'anotherGlobalMapChecker' ), 'Hello', 'get value from global Map via get()' );
+               this.suppressWarnings();
+               assert.equal( window.anotherGlobalMapChecker, 'Hello', 'get value from global Map via window object' );
+               this.restoreWarnings();
+
+               // Change value via global Map
+               globalConf.set('anotherGlobalMapChecker', 'Again');
+               assert.equal( globalConf.get( 'anotherGlobalMapChecker' ), 'Again', 'Change in global Map reflected via get()' );
+               this.suppressWarnings();
+               assert.equal( window.anotherGlobalMapChecker, 'Again', 'Change in global Map reflected window object' );
+               this.restoreWarnings();
+
+               // Change value via window object
+               this.suppressWarnings();
+               window.anotherGlobalMapChecker = 'World';
+               assert.equal( window.anotherGlobalMapChecker, 'World', 'Change in window object works' );
+               this.restoreWarnings();
+               assert.equal( globalConf.get( 'anotherGlobalMapChecker' ), 'Again', 'Change in window object not reflected in global Map' );
 
                // Whitelist this global variable for QUnit's 'noglobal' mode
                if ( QUnit.config.noglobals ) {