Page MenuHomePhabricator

ArticleFeedback - Replace usage of the jquery.appear module in favor of mediawiki.viewport
Closed, ResolvedPublic

Description

ArticleFeedbackv5 uses a deprecated core module, jquery.appear. This should ideally be replaced with the non-deprecated core module mediawiki.viewport.

The instances of the jQuery module being used is in modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js:

jquery.articleFeedbackv5.js (line 2144)
		$.articleFeedbackv5.$holder.appear( function () {
			if ( !$.articleFeedbackv5.isLoaded ) {
				$.articleFeedbackv5.load( 'auto' );
				// Track form impressions
				$.aftTrack.track( $.articleFeedbackv5.experiment() + '-' + 'impression' );
			}
		} );

The method is also referenced in another comment:

jquery.articleFeedbackv5.js (line 2163)
		// Adding hash in url will not scroll down to this id, because the element
		// won't exist until .appear is triggered. Let's just simulate this ourselves.
		if ( '#' + $el.attr( 'id' ) === window.location.hash ) {
			$.articleFeedbackv5.highlightForm();
		}

Event Timeline

jquery.appear has been removed in MediaWiki 1.31

Removed by dcceefd84f3444050438851e7518be0a3d59de94 in October 2017.

Change 446355 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] ArticleFeedbackv5 is broken

https://gerrit.wikimedia.org/r/446355

Change 446355 merged by jenkins-bot:
[integration/config@master] ArticleFeedbackv5 is broken

https://gerrit.wikimedia.org/r/446355

Change 493635 had a related patch set uploaded (by Jack Phoenix; owner: Jack Phoenix):
[mediawiki/extensions/ArticleFeedbackv5@master] Bring back jquery.appear

https://gerrit.wikimedia.org/r/493635

We're currently having to hack in the presence of jquery.appear into the extensions. More details here:

https://github.com/AgileVentures/SocialPrescribingWiki/pull/77/files

Is there a simpler approach to fix this? Or is the fix just upgrading to mediawiki 1.32? waiting for mediawiki.viewport to be added in?

Change 493635 merged by jenkins-bot:
[mediawiki/extensions/ArticleFeedbackv5@master] Bring back jquery.appear

https://gerrit.wikimedia.org/r/493635

given that the fix has been merged https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/ArticleFeedbackv5/+/493635/ what's the flow for it being released? will it ever make 1.31? Is it in 1.32? or maybe it will be available in some future version?

Just needs cherry picking to those branches and merging

No there's no "release" particularly, just it being in the release branch to be grabbed using ExtensionDistributor or similar

Change 494790 had a related patch set uploaded (by Reedy; owner: Jack Phoenix):
[mediawiki/extensions/ArticleFeedbackv5@REL1_32] Bring back jquery.appear

https://gerrit.wikimedia.org/r/494790

Change 494791 had a related patch set uploaded (by Reedy; owner: Jack Phoenix):
[mediawiki/extensions/ArticleFeedbackv5@REL1_31] Bring back jquery.appear

https://gerrit.wikimedia.org/r/494791

I'm not particularly impressed with the current version numbers in those branches and bumping them to cause some confusion...

Change 494791 merged by jenkins-bot:
[mediawiki/extensions/ArticleFeedbackv5@REL1_31] Bring back jquery.appear

https://gerrit.wikimedia.org/r/494791

Change 494790 merged by jenkins-bot:
[mediawiki/extensions/ArticleFeedbackv5@REL1_32] Bring back jquery.appear

https://gerrit.wikimedia.org/r/494790

ashley claimed this task.
ashley added a subscriber: Krinkle.

I'm not particularly impressed with the current version numbers in those branches and bumping them to cause some confusion...

Versioning is confusing anyway!

I'm closing this as RESOLVED although given how the title is worded, INVALID would be more appropriate because jquery.appear wasn't replaced with mediawiki.viewport but instead I brought back jquery.appear from the dead. In any case, AFTv5 should now be working again as intended, so it's something!

I'll note that @Krinkle made a good suggestion in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/ArticleFeedbackv5/+/494791/ which I'll want to implement in master in the near future regarding the naming of the "new" RL module.

Thank you for the fix and for the backports to REL1_31 and REL1_32.

I have filled T218759 to enable CI again.