Page MenuHomePhabricator

jQuery 3 migration error in mw.widgets.MediaSearchWidget.prototype.lazyLoadResults
Closed, ResolvedPublic1 Estimated Story Points


(Separated from T160802)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

I'm unfamiliar with this code, not sure how big of an issue this is, but the culprit is in here:

		// Lazy-load results
		for ( i = 0; i < items.length; i++ ) {
			elementTop = items[ i ].$element.position().top;
			if ( elementTop <= position && !items[ i ].hasSrc() ) {
				// Load the image
				items[ i ].lazyLoad();

With jQuery 1, if items[ i ].$element is unattached, .position() simply returns { top: 0, left: 0 }. So effectively this check does nothing and we end up always lazyloading all images. Simply removing the check would solve this issue, but maybe it should be fixed properly?

Jdforrester-WMF moved this task from To Triage to TR0: Interrupt on the VisualEditor board.
Jdforrester-WMF set the point value for this task to 1.

The lazy-load should happen after the elements are attached, ideally, I'm a little confused about why it's called before that happens. I'll try to take a look.

Change 346645 had a related patch set uploaded (by Mooeypoo):
[mediawiki/core@master] MediaSearchWidget: Listed to "change" event to reposition

Change 346645 merged by jenkins-bot:
[mediawiki/core@master] MediaSearchWidget: Listen to "change" event to reposition