Page MenuHomePhabricator

Merge Extension:Theme into core
Open, Needs TriagePublic

Description

This is a proposal to merge the Theme extension into MediaWiki core as built-in functionality that skins can implement in a simple and consistent manner, enabling user- or site-selection of css variations of a given skin, such as a dark version or a layout with more colours.

Background

The terms 'skin' and 'theme' are often used interchangeably, as different products will tend toward one or the other for the same general purpose - a specific styling of the user interface/layout for the product. While these both hold true for MediaWiki as well, 'skins' and 'themes' are each specific things within this context:

Skins: MediaWiki extensions implementing the overall page, including general layout and placement of content and tools, in order to construct and output the final html, css, and js that visitors receive

Themes: Variants of a skin in terms of colours, fonts, simple layout changes; specific to a given skin

  • Consist of css and/or a set of LESS variables to override or replace a skin's default styles
  • Examples: Night mode versions of skins, simplified layouts, different colours on the same general page structure (such as the original green, blue, red, and white versions of BlueSky)
  • Implemented by Extension:Theme to allow skins to specify alternate stylesheets that load over, or instead, of the default stylesheets for the skin

Skins a built-in specified part of MediaWiki core, and so are relatively consistent and straight forward, but themes are currently only implemented in a third-party extension and (inconsistently) in a few specific skins. Because of this, we see repeated requests for use of the functionality both on larger projects like Wikimedia and among smaller third-party projects, but for the most part little comes of it. What implementations we have seen are varied and scattered.

Expressed or demonstrated demand for theme functionality:

Problem

The longstanding desire for this functionality and the problems with the existing skinning system have resulted in several issues:

  1. Fragmentation of implementation methods:
    • Skin developers may not want to depend on a separate extension, and roll their own solutions (sometimes based on the extension, sometimes not)
    • no standard approach, so not standard way to fix/update them as core changes - maintenance nightmare
    • Even if the solution built-into the skin is initially based on and compatible with the Theme extension, still a maintenance nightmare as they fall out of date
    • caching issues in different implementations? ashley please comment, does ext:theme have problems with this?
  2. difficult to implement due to having to install extra things for support and/or start from scratch:
    • developer solutions tend toward front-end JS- and CSS-only overrides instead of working with some of the newer, cleaner technologies (such as LESS variables)
  3. override approaches are inconsistent; need a consistent approach defined in the first place to head toward cleaner solutions (defining colours/variables instead of overriding them to determine skin behaviour)

We don't even know about most of the variants floating around, customised wikis that do a change and then we never hear back about it.

  • stuff like ZeldaWiki.org's default color scheme (aka the "dark" theme for both MonoBook and Vector) -- born out of a certain necessity, made for a certain site but not necessarily targeting the wider MW platform, "just" the users of that one site. For ones like this, such a significant desk and development undertaking that it's precisely the kind of thing we want to shed light on and perhaps even get those developers to participate in the wider community (="new developers" buzzword and all that jazz)
  • Going forward, this should not require an all-out restyle in Skin.css, either. Needs to be easier for the users to do in the first place.

Proposal

  1. Merge Extension:Theme's logic into core, and keep any existing themes not being merged into the skins themselves in the extension, repurposing it as a compilation of the legacy themes.
  2. Migrate toward simpler skin handling from there, once we have consistent basic backend/core support.

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/465451/ per task T122924

Current Theme functionality is as follows:

  • Enable skins to specify alternate stylesheets to load depending on the theme selected (each theme functions as an RL module tied to the name of the theme). These can be loaded over general CSS or instead. Preferred implementation is of course to keep it all as simple as possible and avoid using the theme as an override, and instead contain only the relevant styles for the theme, but this does not preclude developers making a mess of things regardless.
  • Add settings (site and user) for each theme, displayed for users in their preferences when they enable the skin the themes pertain to.

Merging this into core will allow us to then iterate from there:

  • Create a core framework for setting skin variables which will then automatically load into the rest of the skin (or perhaps even core) less.
  • Create onwiki interfaces for site admins to create their own themes to customise the site.
  • Enable skin creators to create css-only skins, reusing existing skins' code without having to worry about maintenance by creating only the RL modules? (No more vector/monobook forks in order to only redo the css.)

We may need to come up with guidelines as to what canned themes skins generally come with - perhaps keep it to only three or so, a light, a dark, and optionally some sort of colourful version, where from there site admins have reasonable examples from which to create their own with the specific colours they actually want.

Concerns

  • That this could cause skin options to explode: more to test against, more common configurations that developers need to not break.
    • There are already infinite things wikis can do with their own styles/gadgets/whatever, so this, if anything, should help clean that up by providing more consistent predictable options to test against, and putting all the options more in the same place...
    • Pandora's box has already been opened, long, long ago. See below (#Precedent).
  • About the addition of more preference items: general concern about too many options causing issues for users, for database management, whatever.
    • From a UX standpoint it should be fine as new options will only be shown for the active skin. Is actually an improvement over status quo as we are more likely to have more inconsistent skin settings at present, and this should help standardise where and how they appear (won't be elsewhere under #Appearance, won't have themes floating around Gadgets)...
    • DBA-side, should be relatively negligible.

Precedent

  • Basically like allowing mw to have extensions and skins in the first place, where the benefit of those is a simpler, more consistent implementation of what is/was already out there; likewise these are also already out there, so a single standard in core keeps them consistent and simplifies the lot.
  • MediaWiki:Skin.css, $wgAllowUserCss (and $wgAllowUserJs) are things and have been things for a long time and as long as they've been things users have created custom CSS color schemes and stuff and shared 'em and now we want them to become somewhat standardized and maintained...
  • Extension:Gadgets can be used to implement this wiki-side, but likewise has issues with code review/maintenance across projects. These are more specific (specific to a skin, in fact), so we're proposing to use git/gerrit for skins' themes in the skin repositories, which is both neater in terms of UX (skin-related stuff showing up with specific skin as opposed to in massive list of usually skin-agnostic gadgets) and review/maintenance/development.

This task now an RfC to merge a patch, because People Were Concerned. For a full overview of why we want to do this, including the background of themes, skins, and what all that means, please see the writeup on mw.org.

What we're proposing is this:

  1. Merging the linked patch to merge Extension:Theme into core. Under wider review, do we have consensus (or at least no objections)? Does anything need changing?
  2. We're also a bit fuzzy at this point what we actually intend next, so further discussion of that is probably very much in order here as well. That, or we can just worry about that later when someone has more time/some actual resources to do... whatever it is. At least putting this much in core means we'll have much more consistent handling across skins regardless of what we do next, as they will not longer need to depend on the extension or roll their own.

Some discussion has also occurred on the related talk page onwiki, but let's continue future discussion here for consistency, as this was the original task for the patch, and phabricator is just less annoying in general.

Event Timeline

Isarra created this task.Jan 5 2016, 10:01 PM
Isarra updated the task description. (Show Details)
Isarra raised the priority of this task from to Needs Triage.
Isarra added a project: MediaWiki-Interface.
Isarra added subscribers: Isarra, ashley, Legoktm.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptJan 5 2016, 10:01 PM
matmarex renamed this task from Implement extension:theme in core with less variable support to Implement Extension:Theme in core with Less variable support.Jan 7 2016, 11:49 PM
matmarex set Security to None.
This comment was removed by SamanthaNguyen.
SamanthaNguyen renamed this task from Implement Extension:Theme in core with Less variable support to Implement Extension:Theme in core.
SamanthaNguyen added a project: Technical-Debt.

Splitting off, there's a separate ticket for supporting Less on-wiki at T56864.

Change 465451 had a related patch set uploaded (by Jack Phoenix; owner: Jack Phoenix):
[mediawiki/core@master] [WIP][DNM] Merge the Theme extension into core

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

Isarra updated the task description. (Show Details)
Isarra renamed this task from Implement Extension:Theme in core to Merge Extension:Theme into core.Dec 6 2018, 1:14 AM
mobrovac updated the task description. (Show Details)
mobrovac moved this task from Inbox to Watching on the TechCom board.
mobrovac added a subscriber: mobrovac.

I updated the task description to reflect the fact that the canonical place for this RfC is its MW page.

mobrovac moved this task from Inbox to Under discussion on the TechCom-RFC board.Dec 12 2018, 1:46 PM
Isarra updated the task description. (Show Details)Feb 7 2019, 12:52 PM

Perhaps this better explains what we actually want to do here? I don't even know how to properly explain any of this anymore.

Basically this RfC is really just us asking if there's any particular reason not to merge the associated patch, once fully tested and such, because it's slightly larger than the usual change.

Jdforrester-WMF added a subscriber: Jdforrester-WMF.

I've copy-pasted the content from MW.org per the request of TechCom. Not linked or re-formatted except where strictly necessary; copy-edits welcome.

Isarra added a comment.Feb 8 2019, 7:33 AM

The links are kind of important.

Tgr updated the task description. (Show Details)Feb 17 2019, 6:51 AM
Tgr added a subscriber: Tgr.Feb 17 2019, 7:03 AM

Linkified.

It's not clear to me whether this is about providing a way for skins to define their own subthemes (so if, say, the Vector maintainers want a night mode, they don't have to worry about coming up with their own settings widget, siteinfo API extension etc, and maybe extensions which provide skin overrides or Vector can make them theme-specific), or about providing a way to define alternative themes outside of the skin (so if I want Vector but dark, I can write Skin:VectorNightMode which overrides a bunch of things in Vector)? The patch seems to be leaning towards the latter.

In the first case, this sounds like a commonsense change to core, except there is no point in exposing the theme array as a configuration variable; skins can just declare their skins statically. Also a lot of the examples in the task description would not be relevant (e.g. this would not replace gadgets).

In the second case, as others have said it sounds like a maintenance nightmare waiting to happen, unless skins can provide a well-defined restricted interface that's available to themes for tweaking (a set of LESS variables being the obvious implementation).

It's not clear to me whether this is about providing a way for skins to define their own subthemes (so if, say, the Vector maintainers want a night mode, they don't have to worry about coming up with their own settings widget, siteinfo API extension etc, and maybe extensions which provide skin overrides or Vector can make them theme-specific)

This is what we're proposing, yes.

or about providing a way to define alternative themes outside of the skin (so if I want Vector but dark, I can write Skin:VectorNightMode which overrides a bunch of things in Vector)? The patch seems to be leaning towards the latter.

This is also required for backward compatibility, as currently it is the only thing we have - eventually we'll want to migrate most of it to be the former only, or defined specifically on-wiki or such, but that will require massive overhauls of not just mw core, but also and especially all of the skins themselves.

In the first case, this sounds like a commonsense change to core, except there is no point in exposing the theme array as a configuration variable; skins can just declare their skins statically.

Why not? One of the main use cases here is skin developers creating a variant of an existing skin instead of an entire new skin. Don't we need to expose the theme array so they can actually register these new variants as available and selectable themes?

Also a lot of the examples in the task description would not be relevant (e.g. this would not replace gadgets).

How is it not relevant that it will provide a neater way to do something that is currently sometimes done via gadgets?

In the second case, as others have said it sounds like a maintenance nightmare waiting to happen, unless skins can provide a well-defined restricted interface that's available to themes for tweaking (a set of LESS variables being the obvious implementation).

You're right, this is a maintenance nightmare, except it's not waiting to happen, it's where we are currently. Only it's even worse than you seem to be imagining, because we're dealing with many different, usually unmaintained, different implementations of the base logic as well, because there is no standard.

Tgr added a comment.Feb 18 2019, 8:44 PM

This is also required for backward compatibility, as currently it is the only thing we have - eventually we'll want to migrate most of it to be the former only, or defined specifically on-wiki or such, but that will require massive overhauls of not just mw core, but also and especially all of the skins themselves.

You mean, backwards compatibility with the Theme extension? Just leave the extension as it is for now, and remove themes from it when they are migrated into the parent skin's repository (assuming the skin maintainers are willing to take ownership).

Why not? One of the main use cases here is skin developers creating a variant of an existing skin instead of an entire new skin.

What I'm saying is, you have two different use cases here (themes added in the main skin's code vs. themes added externally) with somewhat different technical requirements. IMO it would help to set them apart more clearly as IMO one of them is uncontroversial and the other, not so much.

You're right, this is a maintenance nightmare, except it's not waiting to happen, it's where we are currently. Only it's even worse than you seem to be imagining, because we're dealing with many different, usually unmaintained, different implementations of the base logic as well, because there is no standard.

If people install unmaintained or unmaintainable skins and screw up their sites, that's their problem. (We don't always do a good job of warning people what's unmaintained, but that's a whole another discussion.) I'm more wary of creating an expectation that it's a skin maintainer's responsibility to not make any changes that would break other skins built on top of theirs, which I don't think is happening currently, but wouldn't be an unreasonable expectation for a core feature.

Perhaps some history of how skins are will help clarify things here:

  1. Prior to 2014, skins as skins fell into two categories: core skins, and non-core skins. This wouldn't necessarily have been an issue, except this was well prior to MediaWiki's current registration system, and because the core skins were simply directly integrated into MediaWiki itself (skin messages were in the main mw messages directory, rendering functions were scattered across various skinning and output classes and were often skin-specific), they were not even using the same interfaces as the non-core skins to begin with. This caused a few rather significant problems:
    1. Non-core skins simply couldn't do as much as core skins because the interfaces they had available didn't provide the same functionality.
    2. Having separate interfaces for core and non-core skins resulted in the non-core ones going generally unmaintained because they were considered secondary to the actual core skins.
    3. Core skins were much more difficult to develop on and maintain because of the barrier to entry, requiring core access and knowledge of where each individual skin kept its parts, as they weren't exactly consistent.
    4. The quantity of available core skins was limited, not just because of the higher barrier to entry, but also just because we really wouldn't want to have that many default options to begin with. Indeed, the latter still applies - we only package five or so skins with the MediaWiki release tarballs, despite there being many more available that third-parties could potentially use - but now it's also much easier to drop in any other as well. The included ones are merely the defaults, whereas before they were at times the only viable options.
  2. In 2018, I rewrote the PHP source of the MonoBook skin, maintaining consistent HTML output so as to avoid messing anything up user-side. Unfortunately, this still broke the Modern skin, which directly depended on MonoBook's template in order to generate the HTML. Given that Modern was essentially a retheme of MonoBook (same HTML, different CSS), this clearly demonstrates the issues with this approach, regardless of how exactly it's done:
    1. By depending on another skin directly, such as by extending the template class (less common, but what Modern did): new skin remains largely stable with core, and inherits most updates to continue working with core that are implemented in the parent, unless they change the structure of the class itself. Does require said other skin to be installed. Completely breaks if the parent structure ever changes.
    2. By forking another skin (more common; possibly even the most common way to create a new skin currently): new skin does not continue to depend on the original skin; does not break when the original skin changes its structure. Requires ongoing maintenance to continue working with core updates, due to constant breaking changes in core, and otherwise just breaks.

For 1, this shows a lot of what we want to avoid doing here, but also the importance of properly supporting extensibility - one of the main problems with the core skins was the lack of a consistent system behind them that could be used by all skins, and as a result not only did the core skins suffer, third parties could also not effectively make their own (and when they did, they were really prone to breaking - yes, even worse than currently), which was a bit of a problem since generally speaking most sites actually don't want to look like Wikipedia. They have their own brands, and stuff.

2 demonstrates one of the main use cases here - yes, everyone wants accessibility options and night mode and all that, but when it comes to third-party projects, most of the time what they actually want is more to just change the base colours of a skin to, well, be their colours, than make a new one. To make an existing skin follow their own branding. So providing a more stable way of going about that means they're much less likely to get blocked on some old mw missing all the security updates because they lack the time or know-how to maintain a skin themselves that shouldn't even be a separate skin in the first place.

Isarra updated the task description. (Show Details)Feb 19 2019, 11:15 AM
Isarra updated the task description. (Show Details)Feb 19 2019, 11:29 AM
Bawolff added a subscriber: Bawolff.EditedFeb 19 2019, 11:49 AM

So summarizing a discussion I had with @Isarra on irc [I am not a skin expert. I may have misunderstood some of the finer points here. If I make mistakes or say something stupid here, that's my bad]:

Also a lot of the examples in the task description would not be relevant (e.g. this would not replace gadgets).

its not meant to replace gadgets in the sense of allowing user defined styling. The gadget examples are listed to show demand for a feature to make skin variants. This is a proposed API to make skin variants that is believed to be a much better fit than gadgets are for making skin variants, thus replacing a current usage of the gadgets interface with something else.


So its expected there would be two types of skin variants (themes):

  • (for lack of a better word) "core" variants which are bundled with the skin, and developed along side it. For example a "night mode" version of Vector. It is expected such themes would have wide applicability
  • External themes. These are not developed alongside the skin, but would be installed via separate extensions and have a much more limited audiance. For example, if you use mediawiki as part of your company website, maybe you write a theme to take monobook and replace the colours with your corporate colours, and otherwise add branding. Perhaps you want to do something weird that makes sense to your community but monobook devs wouldn't want to bundle for being too special-purpose. For example, maybe you want pink-coloured monobook. Or whatever.

@Tgr has pointed out that there are some concerns about stability expectations around the theme interface. This may result in making it difficult to refactor skins as people would worry about breaking theme's which could potentially do anything. I believe (not 100% sure on this) that the stability contract that @Isarra envisions for this feature is that the interface is unstable (I mean unstable as in the skin can change to break themes. The theming api itself remains stable). At best developers make a best effort to not screw over theme developers. (This is similar to core's existing deprecation policy in practise). While in an ideal world we would have a stable interface, in practise it seems like it would be impossible to make a stable interface that is also flexible enough to meet the usecase of this feature. In particular, even though the ideal would be to have only a set of less variables exposed, there are no existing skins that have flexible enough less variables to make this work. @Isarra tried to design "timeless" so it would work in this fashion, but ultimately using less variables for themeing that skin was insufficient and themes for it has to use css overrides. Furthermore, @Isarra believes that the existing framework (or lack thereof) of people forking skins, using css gadgets, etc is very difficult to maintain; the wide interface provided by themes, while not ideal, would nonetheless be a major improvement on the maintainability front.

So its expected there would be two types of skin variants (themes): ...

This. And the two types of themes must be implemented the same way, using the same interfaces, else we risk a repeat of the mess with the core vs non-core skins back in the day. The only difference can be where they are defined. (So we want to support themes being easily moved or forked from one group to the other, and same packaging regardless of type)

It's not currently anyone's expectation that anything be stable with skins, to my knowledge, but most skin maintainers themselves do indeed try to maintain considerable output consistency for the sake of supporting user-side customisation (scripts and styles) as well as not messing up extension expectations. And even if they do wildly change a skin out from under an existing theme, it being a theme still limits the breakage to be some css needing rules updated instead of mw fatal erroring.

Seriously. The status quo is fatal errors. It sucks.

Change 465451 had a related patch set uploaded (by Jack Phoenix; owner: Jack Phoenix):
[mediawiki/core@master] [WIP][DNM] Merge the Theme extension into core

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

Hi, how do we move this forward, in any direction?

At this point, I would really like to see the associated patch merged, once fully tested and any specific implementation issues are resolved. While I am hoping we'll be able to replace the specific code and logic as part of T217158 later, it would still be incredibly useful right now to be able to establish better practices in terms of existing skins and resolve the current theme code fragmentation where it's been half-merged into a few of them (and would even help with any eventual migration to a better system later just because they'd be less of a mess to begin with at that point). I also firmly believe we can benefit greatly from having a framework in place for testing and implementing the skin variants our users on Wikimedia projects have been asking for for years, because in order to do this effectively, we need something better than what we have. (See for reference: all the past attempts that eventually just broke because they were all done as hacks on top of the skins, as opposed to built-into the architecture of the skin itself.) I have been meaning to showcase such functionality in Timeless pretty much since the beginning of the project, but without any backend support, it has proven completely infeasible to do so in any way that is maintainable.

Are there any specific objections to merging the patch, on a theoretical level or with specific code concerns? Does this lack of response mean we can move forward with this, or what?

Jc86035 added a subscriber: Jc86035.Apr 8 2019, 3:55 PM