XenoRyet has submitted this change and it was merged.
Change subject: Fallback for browsers that don't support Object.defineProperty
..
Fallback for browsers that don't support Object.defineProperty
Bug: T112590
Bug: T112552
Change-Id: I2fdfda115b1557d6f3c201c98705140f4105877d
---
M resources/subscribing/ext.centralNotice.display.js
M tests/qunit/subscribing/ext.centralNotice.display.tests.js
2 files changed, 32 insertions(+), 5 deletions(-)
Approvals:
XenoRyet: Looks good to me, approved
diff --git a/resources/subscribing/ext.centralNotice.display.js
b/resources/subscribing/ext.centralNotice.display.js
index 7303de6..f3decc3 100644
--- a/resources/subscribing/ext.centralNotice.display.js
+++ b/resources/subscribing/ext.centralNotice.display.js
@@ -101,6 +101,27 @@
}
/**
+* Set up the legacy cn.data property using a getter, or a normal
property
+* (for browsers that don't support getters).
+*/
+ function setUpDataProperty() {
+
+ if ( typeof Object.defineProperty === 'function' ) {
+
+ Object.defineProperty( cn, 'data', {
+ get: function() { return
cn.internal.state.getData(); }
+ } );
+
+ } else {
+
+ // FIXME For browsers that don't support
defineProperty, we don't
+ // fully respect our internal contract with the state
object to
+ // manage data, since we assume the object reference
won't change.
+ cn.data = cn.internal.state.getData();
+ }
+ }
+
+ /**
* Expose a promise object to be resolved when the banner is loaded.
*/
function setUpBannerLoadedPromise() {
@@ -183,6 +204,11 @@
// This will gather initial data needed for selection and
display.
// We expose it above via a getter on the data property.
state.setUp();
+
+ // Because of browser limitations, and to maintain our contract
among
+ // components of this module, we have to do this here.
+ // TODO do this some other way...
+ setUpDataProperty();
// Below, we explicitly pass information from state to other
// internal objects, which are not allowed to have dependencies.
@@ -466,6 +492,7 @@
.fail( cn.internal.state.setInvalidGeoData )
.always( function() {
cn.internal.state.setUpForTestingBanner();
+ setUpDataProperty();
setUpBannerLoadedPromise();
fetchBanner();
} );
@@ -591,11 +618,6 @@
$.extend( mw.centralNotice, cn );
cn = mw.centralNotice; // Update the closured-in local variable
}
-
- // $.extend doesn't copy getters, so we have to add our getter like so
- Object.defineProperty( cn, 'data', {
- get: function() { return cn.internal.state.getData(); }
- } );
// Set up deprecated access points and warnings
mw.log.deprecate( window, 'insertBanner', cn.insertBanner,
diff --git a/tests/qunit/subscribing/ext.centralNotice.display.tests.js
b/tests/qunit/subscribing/ext.centralNotice.display.tests.js
index 89f9abf..3a3704b 100644
--- a/tests/qunit/subscribing/ext.centralNotice.display.tests.js
+++ b/tests/qunit/subscribing/ext.centralNotice.display.tests.js
@@ -122,6 +122,11 @@
},
];
+ // Make this property configurable before the first time it's set,
+ // so the browser lets us define it again and again (since public
objects
+ // aren't re-created between tests).
+ Object.defineProperty( mw.centralNotice, 'data', { configurable: true }
);
+
QUnit.module( 'ext.centralNotice.display', QUnit.newMwEnvironment( {
setup: function () {
--
To view, visit https://gerrit.wikimedia.org/r/238326
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2fdfda115b1557d6f3c201c98705140f4105877d
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: master
Gerrit-Owner: AndyRussG
Gerrit-Reviewer: AndyRussG
Gerrit-Reviewer: Awight
Gerrit-Reviewer: Cdentinger
Gerrit-Reviewer: Ejegg
Gerrit-Reviewer: Krinkle
Gerrit-Reviewer: Ssmith
Gerrit-Reviewer: XenoRyet
Gerrit-Reviewer: jenkins-bot <>
___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
