AndyRussG has uploaded a new change for review.
https://gerrit.wikimedia.org/r/233033
Change subject: QUnit: call reallyInsertBanner() due to async issue
......................................................................
QUnit: call reallyInsertBanner() due to async issue
Change-Id: I0c35503687985cad8f3180b0c8ece68b62c56aec
---
M resources/subscribing/ext.centralNotice.display.js
M tests/qunit/subscribing/ext.centralNotice.display.tests.js
2 files changed, 61 insertions(+), 47 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CentralNotice
refs/changes/33/233033/1
diff --git a/resources/subscribing/ext.centralNotice.display.js
b/resources/subscribing/ext.centralNotice.display.js
index afc4d9c..845426d 100644
--- a/resources/subscribing/ext.centralNotice.display.js
+++ b/resources/subscribing/ext.centralNotice.display.js
@@ -152,51 +152,6 @@
function reallyInsertBanner( bannerJson ) {
- var state = cn.internal.state,
- shownAfterLoadingBanner = true,
- bannerLoadedButHiddenReason,
- tmpData;
-
- // Inject the banner HTML into the DOM
- injectBannerHTML( bannerJson.bannerHtml );
-
- bannerLoadedDeferredObj.resolve( cn.internal.state.getData() );
-
- // Process legacy hook for in-banner JS that hides banners after
- // they're loaded and/or adds data to send to
Special:RecordImpression.
- // Only do this if bannerNotGuaranteedToDisplay is set.
- if ( state.getData().bannerNotGuaranteedToDisplay ) {
- if ( typeof cn.bannerData.alterImpressionData ===
'function' ) {
-
- // Data from state is considered read-only.
This legacy hook
- // may add a 'reason' property to the object it
receives.
- // So we send only a copy of the data and check
the added
- // 'reason' property.
- tmpData = state.getDataCopy();
-
- shownAfterLoadingBanner =
- cn.bannerData.alterImpressionData(
tmpData );
-
- if ( !shownAfterLoadingBanner ) {
- bannerLoadedButHiddenReason =
tmpData.reason || '';
- state.setBannerLoadedButHidden(
bannerLoadedButHiddenReason );
- }
- } else {
- state.setAlterFunctionMissing();
- }
- }
-
- // Banner shown following load (normal scenario)
- if ( shownAfterLoadingBanner ) {
- state.setBannerShown();
- }
-
- // If we're testing a banner, don't call
Special:RecordImpression or run
- // mixin hooks.
- if ( !state.getData().testingBanner ) {
- runPostBannerMixinHooks();
- recordImpression();
- }
}
function recordImpression() {
@@ -358,6 +313,62 @@
cn = {
/**
+ * Really insert the banner (without waiting for the DOM to be
ready).
+ * Only exposed for use in tests.
+ * @private
+ */
+ reallyInsertBanner: function ( bannerJson ) {
+
+ var state = cn.internal.state,
+ shownAfterLoadingBanner = true,
+ bannerLoadedButHiddenReason,
+ tmpData;
+
+ // Inject the banner HTML into the DOM
+ injectBannerHTML( bannerJson.bannerHtml );
+
+ bannerLoadedDeferredObj.resolve(
cn.internal.state.getData() );
+
+ // Process legacy hook for in-banner JS that hides
banners after
+ // they're loaded and/or adds data to send to
+ // Special:RecordImpression. Only do this if
+ // bannerNotGuaranteedToDisplay is set.
+ if ( state.getData().bannerNotGuaranteedToDisplay ) {
+ if ( typeof cn.bannerData.alterImpressionData
=== 'function' ) {
+
+ // Data from state is considered
read-only. This legacy hook
+ // may add a 'reason' property to the
object it receives.
+ // So we send only a copy of the data
and check the added
+ // 'reason' property.
+ tmpData = state.getDataCopy();
+
+ shownAfterLoadingBanner =
+
cn.bannerData.alterImpressionData( tmpData );
+
+ if ( !shownAfterLoadingBanner ) {
+ bannerLoadedButHiddenReason =
tmpData.reason || '';
+ state.setBannerLoadedButHidden(
+
bannerLoadedButHiddenReason );
+ }
+ } else {
+ state.setAlterFunctionMissing();
+ }
+ }
+
+ // Banner shown following load (normal scenario)
+ if ( shownAfterLoadingBanner ) {
+ state.setBannerShown();
+ }
+
+ // If we're testing a banner, don't call
Special:RecordImpression or
+ // run mixin hooks.
+ if ( !state.getData().testingBanner ) {
+ runPostBannerMixinHooks();
+ recordImpression();
+ }
+ },
+
+ /**
* Attachment point for other objects in this module that are
not meant
* for outside use.
*/
@@ -464,7 +475,7 @@
// Insert the banner only after the DOM is ready
$( function() {
- reallyInsertBanner( bannerJson );
+ cn.reallyInsertBanner( bannerJson );
} );
},
diff --git a/tests/qunit/subscribing/ext.centralNotice.display.tests.js
b/tests/qunit/subscribing/ext.centralNotice.display.tests.js
index 185d584..ccc5134 100644
--- a/tests/qunit/subscribing/ext.centralNotice.display.tests.js
+++ b/tests/qunit/subscribing/ext.centralNotice.display.tests.js
@@ -125,7 +125,10 @@
mw.centralNotice.choiceData = choiceData2Campaigns;
mw.centralNotice.chooseAndMaybeDisplay();
- mw.centralNotice.insertBanner( bannerData );
+ // We call reallyInsertBanner() instead of insertBanner() to
avoid
+ // the async DOM-waiting of the latter.
+ mw.centralNotice.reallyInsertBanner( bannerData );
+
assert.equal( $( 'div#test_banner' ).length, 1 );
} );
--
To view, visit https://gerrit.wikimedia.org/r/233033
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c35503687985cad8f3180b0c8ece68b62c56aec
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CentralNotice
Gerrit-Branch: campaign_mixins
Gerrit-Owner: AndyRussG <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
