Page MenuHomePhabricator

Allow skins to use different, custom OOUI themes
Closed, ResolvedPublic

Assigned To
Authored By
matmarex
May 30 2015, 3:43 PM
Referenced Files
F31484380: image.png
Dec 20 2019, 8:44 PM
Tokens
"Burninate" token, awarded by Daegontaven."Like" token, awarded by Lens0021."Party Time" token, awarded by matmarex."Like" token, awarded by Prtksxna.

Description

Allow skins to use different, custom OOUI themes, provided by the skin. This is probably not practical until we do T76632 in OOUI.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Krinkle edited projects, added OOUI; removed MediaWiki-ResourceLoader.
Krinkle added a subscriber: Krinkle.

I don't expect this to require changes in the ResourceLoader framework itself. Removing from that component for now in favour of MediaWiki-Interface.

I don't understand why this task does not block the transition of more and more MediaWiki elements to OOUI. As the most recent example, consider how the "move page" interface looks in MW 1.26 with the ArchLinux skin (compare with 1.25 version). It looks completely out of place and considering this task is still open, I assume there is no way to fix the style from the skin part.

It's pretty easy for skins (or extensions) to hack this in (see ResourcesOOUI.php) but it'd be nice to provide an API, yeah.

Is there a plan to actually resolve this ridiculous issue? Will that be before or after the whole MediaWiki interface is converted to OOUI? From what I can tell, you are deliberately breaking any existing skin not used by the WMF and blocking the creation of new ones.

Lahwaacz raised the priority of this task from Low to High.Apr 12 2016, 7:11 AM

No, there is no plan. As far as I know no one has implemented any custom OOjs UI themes that would use this, and I can't justify to myself spending time to work on it, when in all likelihood no one ever will benefit from it.

I think you might've misunderstood this task – it's not about implementing new designs, or about restoring pre-OOUI forms. It's just about allowing third-party skins that already have a matching third-party OOjs UI theme to use it with MediaWiki. There are no such skins at the moment, to my knowledge. Sorry if that's disappointing.

(By the way, raising priority will generally not cause anyone to work on the task, unless they were already planning to.)

There are no custom OOjs UI themes because even if there were, it would be impossible to use them. I'll bet that even with skins, MediaWiki first enabled the configuration of custom skins and after that, first custom skin was created. I can't justify to myself spending time to work on a custom OOjs UI theme when it can't be even enabled.

I'm not asking you to create any new design or restore pre-OOUI forms, but to allow us to do so ourselves. If we already can, please tell me what I'm missing.

I'll bet that even with skins, MediaWiki first enabled the configuration of custom skins and after that, first custom skin was created.

Actually, no, originally there was no way to add a custom skin. You had to implement skins in the main MediaWiki repository, which is also what I suggest you do with your theme. We'd welcome some more themes in OOjs UI.

If we already can, please tell me what I'm missing.

You can hack the 'getSkinSpecific' function in resources/ResourcesOOUI.php in MediaWiki core to return the right path when $theme is your theme's name, right at the TODO comment.

Alternatively, you can use $wgResourceModuleSkinStyles to specify your own styles for modules named 'oojs-ui-core.styles', 'oojs-ui-widgets', 'oojs-ui-toolbars' and 'oojs-ui-windows', if you don't need any theme-specific JavaScript.

Note that neither of these ways lets you define custom icons, which is a bigger implementation problem here. Your theme will probably end up using the ones from the MediaWiki theme. Or they might be broken. Not sure.

Jdforrester-WMF lowered the priority of this task from High to Low.Apr 29 2016, 6:46 PM

Priority reflects reality, it doesn't set it. Sorry.

Change 343234 had a related patch set uploaded (by Bartosz Dziewoński):
[oojs/ui] Implement grunt add-theme task to ease theme creation

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

Change 343239 had a related patch set uploaded (by Bartosz Dziewoński):
[mediawiki/core] Allow skins/extensions to define custom OOjs UI themes

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

Change 343240 had a related patch set uploaded (by Bartosz Dziewoński):
[mediawiki/skins/Example] [DO NOT MERGE] Example of defining custom OOjs UI theme

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

As of 2019, this comment is outdated. See here for current documentation: https://www.mediawiki.org/wiki/OOUI/Themes

I spent a few days working on this. With the patches above (and their dependencies, in MediaWiki and OOjs UI: https://gerrit.wikimedia.org/r/#/q/topic:custom-ooui-themes) it is actually feasible to create and use a custom OOjs UI theme. I wouldn't say it's easy (for that, we'd need T76632, pretty much rewriting all theme logic), but it's totally doable.

Assuming these get merged without major changes, the steps to create a new OOjs UI theme and use it in your MediaWiki skin will be:

  1. Fork the OOjs UI repository, e.g. from https://github.com/wikimedia/oojs-ui. Run npm install and make sure you can build the library with grunt build and view the demo.
  2. Run grunt add-theme --name=FooBar --template=Blank, substituting your theme's name for FooBar. Rather than use the Blank theme, you can also start with Apex or MediaWiki.
    • This will create a copy of the template theme under a new name, and tweak the build process and demos to include your new theme.
    • Your theme's styles are now in src/themes/foobar/. Hack away! Run grunt build (or grunt quick-build) to build it, use the demo to test your changes.
  3. When your theme is done, it can be added to a skin.
    • Run grunt build --graphics=vector.
    • Copy-paste the generated files from dist/ (you will need oojs-ui-foobar.js and all oojs-ui-...-foobar.css, ignore the others) into a subdirectory in your skin. Also copy FooBarTheme.php from php/.
    • If you have custom images, also copy-paste contents of dist/themes/foobar/ and the .json files from src/themes/foobar/.
    • Add OOUIThemePaths to the skin.json file pointing to these paths, and make your skin use the new theme with SkinOOUIThemes, as in this example: https://gerrit.wikimedia.org/r/343240. Use "images": null if you don't have any.
  4. Regularly merge back the changes from the OOjs UI repository into your fork. We release OOjs UI weekly on Tuesdays. You should probably do it at least for new MediaWiki versions (roughly every 6 months).
    • Although we don't usually make gratuitous changes that would require fixes to theme styles, they sometimes happen, and there probably won't be any warnings. Sorry. You can watch for changes in the Apex theme to spot things that are likely to affect your theme too (Apex is not in active development).
    • Even if there were no such breaking changes, you should still occasionally rebuild your version to include any changes to the base styles.
  5. Eventually, ask us to merge your theme into upstream OOjs UI and include it with MediaWiki!

I am hoping to see my work rewarded with tons of new OOjs UI themes now :)

Change 343234 merged by jenkins-bot:
[oojs/ui@master] Implement grunt add-theme task to ease theme creation

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

Volker_E moved this task from Backlog to OOjs-UI-0.20.1 on the OOUI board.
Volker_E edited projects, added OOUI (OOjs-UI-0.20.1); removed OOUI.
Volker_E removed a project: Patch-For-Review.
Volker_E removed a subscriber: gerritbot.

This isn't resolved, the MediaWiki core patch is still not merged :(

matmarex lowered the priority of this task from Low to Lowest.Apr 16 2018, 11:29 PM

Why isn't this merged? I've created a theme and I just realized that it didn't work because this wasn't merged. How is anyone supposed to create a MediaWiki theme if one organization(WikiMedia) is breaking it and hard coding for itself?

Daegontaven raised the priority of this task from Lowest to High.Jan 12 2019, 11:35 AM
Aklapper lowered the priority of this task from High to Low.Jan 12 2019, 12:20 PM

You are welcome to increase priority and set yourself as task assignee if you plan to work on fixing this, as the Priority field summarizes and reflects reality and does not cause it.

Daegontaven raised the priority of this task from Low to Medium.

@Daegontaven It seems that it simply has not been a priority for anyone who could review and approve it. Which seems understandable to me, as you're the first person in two years to ask about it.

The patch I proposed adds a bunch of configuration options that will have to be supported forever, and makes some changes to ResourceLoader internals to allow loading the themes from other places, so it's not trivial to review.

And there is – or perhaps was – a bit of a chicken-and-egg problem. No one needs this feature until they develop a custom OOUI theme, but no one develops a custom OOUI theme until they can use it… If you're developing one and the approach in my patch works for you, then perhaps that helps validate it and eases the minds of potential reviewers :)

The patch I proposed adds a bunch of configuration options that will have to be supported forever, [...]

Wasn't the original skinnability supposed to be supported forever?

And there is – or perhaps was – a bit of a chicken-and-egg problem. No one needs this feature until they develop a custom OOUI theme, but no one develops a custom OOUI theme until they can use it…

Maybe you should change your prioritization or review process to avoid such chicken-and-egg problems. It is incredibly easy for developers to break something (e.g. by developing OOUI) because they know that reviewers are biased towards the benefits of just one organization, one mindset and one appearance.

Wasn't the original skinnability supposed to be supported forever?

I'm not sure what you mean by "original skinnability". But regardless this is a new addition.

Maybe you should change your prioritization or review process to avoid such chicken-and-egg problems. It is incredibly easy for developers to break something (e.g. by developing OOUI) because they know that reviewers are biased towards the benefits of just one organization, one mindset and one appearance.

I'm not sure what you mean here either. But I will note that OOUI already comes with two themes, both of them are in use on Wikimedia wikis (the alternative theme is named Apex and used by the MonoBook skin), and WMF projects that I know of are tested with both.

If you're developing one and the approach in my patch works for you, then perhaps that helps validate it and eases the minds of potential reviewers :)

I already finished developing a theme. However there is no way for me to make sure it works apart from looking at the demo. How do I go about merging it in if there is hard coding involved ? Will it be accepted If I simply submit the patch for my theme with my theme in the src/themes folder? I am asking all this because I am by no means familiar with ResourceLoader.

To test your theme with MediaWiki:

  1. On your local testing wiki, checkout my proposed patch for mediawiki/core: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/343239 (use the Download → Checkout menu in Gerrit and copy-paste that command, e.g. git fetch https://gerrit.wikimedia.org/r/mediawiki/core refs/changes/39/343239/10 && git checkout FETCH_HEAD)
  2. Add your OOUI theme to the skin where you intend to use it, following this example: https://gerrit.wikimedia.org/r/c/mediawiki/skins/Example/+/343240 and the instructions I wrote above: T100896#3108973

(I just updated both of the patches linked above to be compatible with latest MediaWiki.)

@matmarex I've been able successfuly test out your patch. However this breaks for RTL.

So this works:

"OOUIThemePaths": {
        "Discord": {
                "scripts": "resources/ooui/oojs-ui-discord.js",
                "styles": "resources/ooui/oojs-ui-{module}-discord.css",
                "images": "resources/ooui/{module}.json"
        }
},

However, this doesn't:

"OOUIThemePaths": {
        "Discord": {
                "scripts": "resources/ooui/oojs-ui-discord.js",
                "styles": [
                        "resources/ooui/oojs-ui-{module}-discord.css",
                        "resources/ooui/oojs-ui-{module}-discord.rtl.css"
                ],
                "images": "resources/ooui/{module}.json"
        }
},

Acessing this path displays the issue:
w/load.php?debug=true&lang=en&modules=oojs-ui-core.styles&only=styles&skin=discord

/*
[08f826fbdee98976fb83fe4f] 2019-01-19 06:05:25: Fatal exception of type "MWException"

Problematic modules: {
    "oojs-ui-core.styles": "error"
}
*/

Corresponding apache error log:

[Sat Jan 19 06:05:25.171300 2019] [:error] [pid 5280] [client 108.162.241.221:51582] PHP Notice:  Array to string conversion in /var/www/html/w/includes/resourceloader/ResourceLoaderFilePath.php on line 55
[Sat Jan 19 06:05:25.171385 2019] [:error] [pid 5280] [client 108.162.241.221:51582] PHP Notice:  Array to string conversion in /var/www/html/w/includes/resourceloader/ResourceLoaderFilePath.php on line 62

@Daegontaven Thanks for trying it out!

These values are supposed to be strings, not array. I think you should not need to use the .rtl.css files at all – MediaWiki also automatically "flips" all stylesheets for RTL languages, using the same library that generates the .rtl.css files (CSSJanus). Does the first version (using "styles": "resources/ooui/oojs-ui-{module}-discord.css") not result in correct (flipped) rendering in RTL?

(I improved the validation for OOUIThemePaths in the latest patchset of https://gerrit.wikimedia.org/r/c/mediawiki/core/+/343239, so that running maintenance/validateRegistrationFile.php on your skin.json file will now warn about this.)

I'm trying this patch and it is good with my skin. I am waiting for this merged :)

taven@debug-wiki:/var/www/html/w$ php maintenance/validateRegistrationFile.php ~/mediawiki/Discord/skin.json 
/home/taven/mediawiki/Discord/skin.json did not pass validation.
[OOUIThemePaths.Discord.styles] Array value found, but a string is required

Awesome, thank you so much for this patch. Everything seems to work!

There are a few questions that come to mind now that this is esentially resolved:

  • When will this be merged in?
  • If it doesn't get merged in immediately, what are the conditions that have to be met in order to get it merged in?
  • Will this be backported (atleast 1.32) if it does get merged in since it appears to be compatible with with it?

Thanks. I'm glad it works for you :)

I'll ask some folks to review the patch again. No promises on when it will be merged, sorry. But I hope it will be easier for me to motivate reviewers, now that we know their time will not be wasted on a feature no one would use!

I can submit a patch for backporting, but I'm not sure if it will be accepted. Unfortunately this requires some scary-looking changes to allow loading the files for different themes from different paths.

@Daegontaven @Lorentz21 If your skins' source code is public, can you push the patches using your themes somewhere and link them here, so that we can try out the MediaWiki patch with them as well?

Change 343239 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Allow skins/extensions to define custom OOUI themes

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

Change 343240 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/Example@master] [DO NOT MERGE] Example of defining custom OOUI theme

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

Not really any progress. (I fixed a few merge conflicts in the patches.)

Aklapper lowered the priority of this task from Medium to Low.Jul 11 2019, 10:36 AM

Change 343239 merged by jenkins-bot:
[mediawiki/core@master] Allow skins/extensions to define custom OOUI themes

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

Jdforrester-WMF assigned this task to matmarex.

OK, all things being OK this'll ship with MediaWiki 1.34.0.

Change 343240 abandoned by Bartosz Dziewoński:
[DO NOT MERGE] Example of defining custom OOUI theme

Reason:
This is perfectly functional, but I don't think we should merge it, as skin authors often start by copy-pasting this repo and most skins should not require custom OOUI themes (and especially not custom icon sets).

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

I documented this at https://www.mediawiki.org/wiki/OOUI/Themes, adapting my earlier comment in this task.

Change 525080 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Add release note for custom OOUI themes using OOUIThemePaths (T100896)

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

Change 525080 merged by jenkins-bot:
[mediawiki/core@master] Add release note for custom OOUI themes using OOUIThemePaths (T100896)

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

FYI: FemiWiki has started using a custom OOUI theme.

image.png (450×565 px, 43 KB)