Page MenuHomePhabricator

[performance] Interval not cleared: 2 Intervals per file
Closed, ResolvedPublic

Description

Simply test it: During the whole upload process 2 intervalls *per file* are not cleared:

var se = window.setInterval;
window.setInterval = function (fn, t) {
  return se(function() {
    console.log(fn, t);
    fn();
  }, t)
};

If you upload 50 files, this means 100 events fire (all the time) within 500ms, executing a function that is computing some numbers from the DOM (object position) and setting CSS. Not good for old machines and slow browsers.

Culprit:
/extensions/UploadWizard/resources/mw.UploadWizardUploadInterface.js

moveFileInputToCover: function( selector ) {
//...

this.moveFileInputInterval = window.setInterval(function() {
  update();
  }, 500); 
}

Version: unspecified
Severity: major

Details

Reference
bz37997

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 12:31 AM
bzimport added a project: UploadWizard.
bzimport set Reference to bz37997.
bzimport added a subscriber: Unknown Object (MLST).
Rillke created this task.Jun 27 2012, 7:39 PM

Change 75122 had a related patch set uploaded by Rillke:
Improved file-control positioning

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

Change 75122 abandoned by Rillke:
Improved file-control positioning

Reason:
Too much work to do. No time fixing boring white-space.

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

Change 75122 restored by Alex Monk:
Improved file-control positioning

Reason:
Don't abandon changes for such silly reasons.

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

Rillke added a comment.Aug 3 2013, 1:52 AM

Created attachment 13056
Screenshot showing Firebug, UploadWizard and Mozilla Firefox. CSS has been tweaked to make the input visible.

Attached:

Change 75122 merged by jenkins-bot:
Improved file-control positioning

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

still an issue; timeouts must not be used for adjusting positions; use resize events or/and mutation observers

Rillke updated the task description. (Show Details)Feb 13 2015, 7:32 PM
Rillke set Security to None.
Jdforrester-WMF moved this task from Untriaged to Backlog on the Multimedia board.Sep 4 2015, 6:34 PM
Restricted Application added subscribers: Steinsplitter, Matanya, Aklapper. · View Herald TranscriptSep 4 2015, 6:34 PM
MarkTraceur lowered the priority of this task from Normal to Low.Dec 3 2015, 6:26 PM

I don't believe this is an issue anymore. The interval is cleared once the upload is filled, and a new interval is created in the new empty upload. We could use events to do this, but there's no reason to rush to that right now.

matmarex closed this task as Resolved.Feb 18 2016, 9:08 PM
matmarex claimed this task.
matmarex closed subtask T126712: Kill moveFileInputToCover as Resolved.
matmarex added a subscriber: matmarex.

This should be gone for good with T126712.