Page MenuHomePhabricator

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

Description

(Separated from T160802)

Event Timeline

matmarex created this task.Apr 4 2017, 10:24 PM
Restricted Application added a project: VisualEditor. · View Herald TranscriptApr 4 2017, 10:24 PM
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 triaged this task as High priority.Apr 5 2017, 6:52 PM
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

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

Mooeypoo claimed this task.Apr 10 2017, 6:39 PM

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

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

Jdforrester-WMF closed this task as Resolved.Apr 10 2017, 7:10 PM
Jdforrester-WMF removed a project: Patch-For-Review.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptApr 10 2017, 7:10 PM