Page MenuHomePhabricator

Remove `hasLoadError` instance variable/logic from Overlay/OverlayManager
Closed, ResolvedPublic0.5 Estimated Story Points

Description

Overlay.js contains an instance variable hasLoadError:

function Overlay( props ) {
	this.isIos = browser.isIos();
	// Set to true when overlay has failed to load
	this.hasLoadError = false;
       
 ...
}

This used to be changed by ImageOverlay (before T216198) whenever an image failed to load so that OverlayManager wouldn't cache the overlay. However, this is no longer the case and nothing actually sets this instance variable to true now. Therefore, all logic involving it can be removed from Overlay and OverlayManager.js!

Acceptance criteria

  • The hasLoadError instance variable is removed from Overlay.js
  • All logic related to hasLoadError is removed from OverlayManager.

Event Timeline

Change 503545 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Remove hasLoadError logic from Overlay/OverlayManager

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

Change 503545 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove hasLoadError instance variable/logic from Overlay/OverlayManager

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

Jdlrobson subscribed.

A patch got merged last Wednesday so this task needs to go LTR through the board.

Niedzielski set the point value for this task to 0.5.Apr 29 2019, 5:10 PM
pmiazga triaged this task as Medium priority.

The ImageCarousel class should declare it's own hasLoadError property as it is the only place that still uses it.

Change 507428 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/extensions/MobileFrontend@master] Define hasLoadError instance variable in ImageCarousel

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

Change 507428 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Define hasLoadError instance variable in ImageCarousel constructor function

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