Page MenuHomePhabricator

Gallery styles should only be loaded on pages that need them (desktop and mobile)
Closed, ResolvedPublic

Description

It seems since galleries are invoked by the parser it would make sense to only load the associated styles when a gallery is on the page, given that numerous pages do not have galleries rather than loading them all in the head.

This will help reduce firstPaint time.

Event Timeline

Jdlrobson updated the task description. (Show Details)
Jdlrobson raised the priority of this task from to Normal.
Jdlrobson assigned this task to Gilles.
Gilles removed Gilles as the assignee of this task.Jun 2 2015, 8:21 AM
Krinkle set Security to None.
Krinkle added a subscriber: Krinkle.EditedJun 3 2015, 12:38 AM

Removing ResourceLoader component. The ability to add modules to a page already exists.

For example "mediawiki.toc" (in core) and ext.cite (Cite extension) both make use of this

EDIT: In fact, this has already been done in 2013. Module "mediawiki.page.gallery" is only loaded if the a gallery has been encountered in the page. The TraditionalImageGallery class deals with tracking the needed modules and adding them to the parser output.

So we need to move styles from legacy code to a new module mediawiki.page.gallery.styles which gets loaded at same time as mediawiki.page.gallery

MobileFrontend could then define a skinStyles for that module.

Jdlrobson moved this task from Backlog to Bugs on the MobileFrontend board.Aug 4 2015, 6:38 PM

Change 233083 had a related patch set uploaded (by Jdlrobson):
Only load gallery styles when needed

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

Change 233084 had a related patch set uploaded (by Jdlrobson):
Only load gallery styling rules when gallerys are on the page

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

Change 233085 had a related patch set uploaded (by Jdlrobson):
Do not double load gallery styles now cache has expired

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

I hope this trend is not pushing towards other (generic) classes, such as thumbs. Those are used by templates, and this may apply to galleries as well.

I hope this trend is not pushing towards other (generic) classes, such as thumbs. Those are used by templates, and this may apply to galleries as well.

Maybe there should be a possibility for templates to request special modules.

@Fomafix @Edokter - I'm a little confused. If you use the gallery tag anywhere including inside templates they will load these styles. There's also the workaround of a blank gallery tag.

@Fomafix @Edokter - I'm a little confused. If you use the gallery tag anywhere including inside templates they will load these styles. There's also the workaround of a blank gallery tag.

It's not about the <gallery> tag. It's about templates generating HTML that uses the same css class names as used by the MediaWiki parser for <gallery>, and image thumbnails. This is used in constructs such as Template:Panorama.

Got it thanks for clarification. This seems like a general problem we need to solve. In mean time couldn't a blank gallery tag be added to these templates?

Change 233084 merged by jenkins-bot:
Only load gallery styling rules when galleries are on the page

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

https://gerrit.wikimedia.org/r/#/c/233083/2 is ready for review now the upstream has happened. Thanks again @matmarex.

Right, now we wait 30 days to merge https://gerrit.wikimedia.org/r/233085, and we're done.

/me marks calendar

@Jhernandez: I recommend this gets dropped to low priority while we wait the next 30 days out.

Jdlrobson added a comment.EditedAug 28 2015, 5:09 PM

@phuedx so... we still need to merge the mobile patch with high priority.
Without https://gerrit.wikimedia.org/r/#/c/233083/2 we will start loading desktop styles on the mobile site for galleries (which will not work)

A side effect of this patch is that currently cached HTML featuring galleries will not load the css... I can however make it double load if we want.

Change 233083 merged by jenkins-bot:
Only load gallery styles when needed

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

Jdlrobson changed the task status from Open to Stalled.Aug 28 2015, 7:02 PM

Okay this is now stalled. We need to revisit it in 2 sprints.

Change 236053 had a related patch set uploaded (by Alex Monk):
Load new gallery styles module

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

Change 236053 merged by jenkins-bot:
Load new gallery styles module

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

Jdlrobson closed this task as Resolved.Sep 21 2015, 6:37 PM

Good stuff!

Change 240563 had a related patch set uploaded (by Florianschmidtwelzow):
Load gallery styles on Category pages, too

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

Change 240563 abandoned by Florianschmidtwelzow:
Load gallery styles on Category pages, too

Reason:
See I32697c2c65824d7622c.

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