Page MenuHomePhabricator

ext.imageMetrics.head should only load on File pages
Closed, ResolvedPublic

Description

Instead of loading everywhere.

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added subscribers: Mholloway, Ricordisamoa, He7d3r and 7 others.
Restricted Application added a project: Multimedia. · View Herald TranscriptMay 25 2015, 10:04 PM
phuedx added a subscriber: phuedx.May 28 2015, 1:48 PM

@Jdlrobson: What's it used for? What do we need it for? How do we stop it from loading in MobileFrontend?

Jdlrobson set Security to None.Jun 1 2015, 5:10 PM
Jdlrobson added a subscriber: Tgr.

@Tgr I hear you wrote this. Was enabling it on mobile an oversight or is it serving a purpose?

Tgr added a comment.Jun 1 2015, 6:52 PM

It's collecting image loading times for the big images on file pages, and information about CORS support. The latter needed to be done on mobile as well, but we probably already collected enough data so I'll disable it once I have the time to go through the data and verify it is valid. If you feel the image loading times are not useful on mobile, I can disable that as well. (Mind you, the file is ~200B minified+gzipped, so removing it would change the size of the head scripts by about 0.2%.)

@Tgr despite the size the concern is about whether it needs to be loaded in the head.

Thanks for looking into this.

Tgr added a comment.Jun 1 2015, 8:42 PM

It is top-loaded so that it can capture the load event of the image, which can happen before normal scripts are loaded.

Tgr removed a project: Epic.Jun 1 2015, 8:43 PM
Gilles added a subscriber: Gilles.Jun 2 2015, 9:57 AM

I think the main concern will go away as soon as you make it restricted to the file pages again.

Jhernandez renamed this task from ext.imageMetrics.head shouldn't be loaded on mobile to ext.imageMetrics.head should only load on File pages.Aug 13 2015, 6:30 PM
Jhernandez updated the task description. (Show Details)
Restricted Application added a subscriber: Matanya. · View Herald TranscriptAug 13 2015, 6:30 PM

@Tgr @Gilles I've edited title based on your comment. Could it be reasonable?

Do you want us to help with this next week?

cc/ @Jdlrobson (what do you think about this?)

Jdlrobson closed this task as Resolved.Aug 13 2015, 7:56 PM
Jdlrobson claimed this task.

This is now fixed.

Awesome, thanks @Tgr