Page MenuHomePhabricator

Community discussion about hacks.less
Closed, ResolvedPublic

Description

hacks.less is a file living in the MinervaNeue code base that overrides the styling of some templates and other community-made constructs so that they display more usefully at mobile resolutions. This breaks the expectations that skins should stick to skinning the basics and leaving user-created structures alone. What should be done about the styles in the file?

Making this as a subtask but it could as easily be detached elsewhere.

Outcome

  • Navbox styles are removed. They no longer make sense given the mobile site strips navboxes from the pages, we have solutions for large tables, and many navbox templates now use a div to wrap the table.
  • Scope the page issues styles to Minerva. These don't make sense in Minerva.

Event Timeline

The reason for "why hasn't English Wikipedia turned on (off?) wgMinervaApplyKnownTemplateHacks" should be apparent in the below mix of comments about what is in these files. Some of it is featureful and I think it would be a loss, some of it is 'we're working on it/so close', and some of it is 'get rid of it'.

Besides that, we have a 900 wiki scale issue right now for WMF.

The other reason you maybe don't want to move these to WikimediaMessages specifically is that many other wikis use all the same classes. I feel like MobileFrontend is the better home generally for the hacks that need to be hacks.

/* templates/ambox.less */
@import '../../../../minerva.less/minerva.variables.less';

/**
 * Ambox classes are nested in a top-level class
 * for the page issues A/B test.
 * This class is appended to the DOM via JS
 **/

@amboxPadding: 8px;
@amboxIconPadding: @amboxPadding * 4;

.ambox,
/* Be more specific than .content table styles in Minerva */
table.ambox {
	display: none;
	margin: 0;
}

... Oof. All amboxes hidden everywhere unless JS is available seems like something that can get kicked to the curb. Read below comment for additional context. If this were to be re-added outside that context, there's likely CSS in the below group that would need to be moved out of the JS only selector.

See also T312732 I guess?

// Will show FOUC on mobile
.issues-group-B {
	.ambox {
		display: block;
	}
}

.client-js .ambox {
	/* snip */
}

Some of this goes with the feature that amboxes have on Minerva that causes them to display more content when pressed. I haven't decided whether it's good or bad that Minerva can do this, and I don't know if pushing it out to the Masses in a gadget is a good thing either (c.f. community missing a central software location). Something like this was added in a gadget for Timeless. It probably deserves moving to Somewhere. It strikes me as best placed in MobileFrontend, which I essentially perceive as a hack to make mobile more palatable. But maybe there's differing opinions out there on that extension's purpose.

Some of this does not go with that feature, and is broadly opinion on how amboxes should look on mobile (see also discussion in T316986). There might be some stuff in the pile here that aren't opinion that were amboxes to be displayed would need to be migrated up/out of the .client-js.

Getting amboxes to the point where we don't need custom styles for anything other than the click feature is blocked on someone working on Module:Message_box on en.wp (particularly, getting it all moved over to div + flex/grid, which has been on my to do list).

/* templates/reflist.less */
/**
 * Template:Reflist - the 30em wide, two column list of references on Wikipedia.
 * https://en.wikipedia.org/wiki/Template:Reflist
 **/

.content {
	.reflist {
		column-gap: 2em;
	}
}

I have no idea what this is doing here. If any wiki wanted a larger gap, they would change their CSS for it. Not that gaps are presented on mobile, which is what hacks.less is for, since for the most part the page isn't wide enough to display as two columns.

/* hacks.less */

/*
A file for css that corrects known rendering issues on known Wikimedia wikis.

the following definitions exist to deal with certain inline styles
present in wikitext.
This file should not need to exist
It will become redundant when the following RFC is resolved:
https://www.mediawiki.org/wiki/Requests_for_comment/Allow_styling_in_templates

FIXME: Review all of these hacks to see if they still apply.
*/
@import '../../../minerva.less/minerva.variables.less';
@import '../../../minerva.less/minerva.mixins.less';
@import 'templates/ambox.less';
@import 'templates/reflist.less';

I guess we're doing the FIXME now :). The two imports of interest are covered above.

.collapsible td {
	width: auto !important;
}

Ok, so there's 3 cases I know of for this one?

  1. NavFrame users. Probably not the primary issue with this line since NavFrame was a div based system (which isn't to say NavFrames can't have tables... but that falls through to item 3 below).
  2. Wikis that have .collapsible as a synonym for .mw-collapsible in Common.js. This is where English Wikipedia is, but even we haven't cleaned all the uses up. (Some 10-20k out in the wild just in mainspace.)
    • I assume for these wikis migrating to mw-collapsible fully would fix this issue? That's probably quite a bit of work.
  3. Wikis that don't even have that yet. A lot more work, as my own efforts to deprecate NavFrame alone took.

On the other hand, I'm not sure how valuable this line is; I get the sense that most wikis in the 2 and 3 groups won't have had any JavaScript to make .collapsible function on mobile, so they'd just get the table cell widths defined by Minerva.

Other than that, there's a possibility this is cheating as a singular identifier for many Something Elses... would have to research this in the git history.

.content {
	.vertical-navbox,
	.navbox {
		display: none;
	}

I don't really get what this is doing here in this day and age. Minerva the skin shouldn't care, and these classes furthermore get removed by MobileFrontend.


	// stylelint-disable selector-max-id
	#coordinates,
	/* Hide article badges, clean-up notices, stub notices, and navigation boxes */
	.topicon {
		// It's important as some of these are tables which become display: table on larger screens
		display: none !important;
	}
	// stylelint-enable selector-max-id

Kind of doubtful of the "it's important". I don't think I've seen a top icon do this on English Wikipedia. Maybe other wikis are crazy like this???

Many wikis probably also don't have top icons anymore but use indicators, or should be using indicators... (it's been long enough)? T75299: Indicators (protected icon, featured icon) are not shown in Minerva is relevant I feel like.

Regarding coordinates, wikis have a migration path for that now (English Wikipedia at least figured out how to do it in an indicator, as have other wikis).

	table,
	.infobox {
		// Unfloat tables and infoboxes:;
		// A lot of templates introduce floating and horizontal margins inline styles
		float: none !important;
		margin-left: 0 !important;
		margin-right: 0 !important;
	}

	.infobox { /* snip */ }

}

Regarding table, this probably has to get merged into the tables.less file. It's still a hack though of course. :|

Regarding .infobox, multiple factors going on here at least for en.wp:

  1. The unconcluded effort to get infobox into TemplateStyles and particularly the several thousand pages which have the class but not the template.
  2. Otherwise we're just adding all this CSS to Mobile.css, which is not a win. A solution to this problem which would enable getting this stuff out of here is that T190083: Make MediaWiki:Mobile.css render-blocking gets done done since it's just infoboxes left as the "big" item in English Wikipedia's Common.css, or Common.css gets loaded on mobile... (but also render-blocking) (which doesn't have its own task? you implied it was an end goal at T248416#6046903). I can work toward a goal this direction for en.wp if someone commits to close support (of some sort?). Probably something that could help here is getting a measure of Common.css and Mobile.css on every wiki and then using a config to turn render-blocking on for some set of wikis with a Reasonably Sized CSS page.
// FIXME: Remove when filetoc is stripped from file pages a la table of contents (toc)
#filetoc { // stylelint-disable-line selector-max-id
	display: none;
}

This seems like your own FIXME to fix/verify. Should probably have a task for that somewhere.

/* T36878: Set an optimal width for a column.
 * Makes sure that on small screens column-count is only honored if column-width hint is not violated.
 * https://developer.mozilla.org/en-US/docs/CSS/column-width
 */
.references-column-count,
.column-count {
	-moz-column-width: 35em;
	-webkit-column-width: 35em;
	column-width: 35em;
}

English Wikipedia almost has these as a non-issue now, having removed the options to provide a column count (n.b. if both count and width are supplied, then browsers treat the count as the maximum count rather than the absolute count, so that's why this hack works).

// When JavaScript is disabled clarify to the user which reference they clicked on
.references li:target {
	background-color: var( --background-color-progressive-subtle );
}

I'm... pretty sure Cite has this in its siteStyles already? If not in Cite, then other skins are taking care of it, so you should verify if this is in the wrong place. (It probably wants a .client-nojs too if it stays in Minerva.)

.hatnote,
.dablink,
.rellink { /* snip */ }
}

This is an opinionated idea of what hatnotes should look like on mobile and I'd be happy to see it removed post-warning to wikis about whether they want it.

@media all and ( min-width: @width-breakpoint-tablet ) {
	// When in the HTML these should be revealed at tablet resolution (T172078)
	.content {
		.vertical-navbox,
		.navbox {
			display: inherit;
		}
	}
}

See above.

// Hacks to render galleries and multicol tables better on mobile
@media all and ( max-width: @width-breakpoint-tablet ) {
	.content {
		table {
			// Make {{col-beg}}, {{col-break}}, and {{col-end}} templates display single column tables
			&.multicol {
				> tr > td,
				> tbody > tr > td {
					display: block !important;
					width: auto !important;
				}
			}
		}

We can probably fix this on wiki in some way or another.

		// Deal with Template:Multiple_image.  T38030 and T148505
		.thumb .thumbinner {
			.flex-display( flex );
			justify-content: center;
			flex-wrap: wrap;
			align-content: flex-start;
			// avoid image child overflowing the container (T200518)
			// stylelint-disable-next-line declaration-block-no-redundant-longhand-properties
			flex-direction: column;

			> .thumbcaption {
				.flex( 1, 0, 100% );
				display: block;
			}
		}
	}
}

This is definitely somethng we can fix and just needs a closer look to verify that it's still working the way it's supposed to onwiki. That said, these styles are applying just about everywhere there are thumbs I think??? (not even galleries as I think multiple_image now uses, thumbs!) and so IDK if your images.less file is handling these correctly.

In general, I'd like to get to a place where community members can cherry pick the things they want. I'll share more on this when I have time to collect my thoughts but wanted to jump in and say thank you for creating this!

Hey @Izno if we were to flesh out https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikimediaMessages/+/1007990 (which has examples for hatnote and navbox templates) I think this would help us gradually chip away at this so that you get the styles you want and have the power to turn off the styles you don't via a new configuration flag. For example

# Would disable navbox and hatnote styles (once the hacks.less file has been carved up so they are in separate files 
$wgWikimediaStylesExcludeFeatures =  [ "navbox", "hatnote"];
# For projects that want us to continue to patch their templates to make them work as they don't have capacity to fix them themselves.
$wgWikimediaStylesExcludeFeatures =  [];

As part of this work we'd also clean up the FIXMES such as #filetoc and the Cite styles. I think this exercise would be a good opportunity to revise each of these as you've outlined in detail.

What do you think? Is this worth pursuing ? Would you use it?

In general, I'd like to get to a place where community members can cherry pick the things they want. I'll share more on this when I have time to collect my thoughts but wanted to jump in and say thank you for creating this!

I had a response to this one typed up before the POCs showed up, but I'll merge it with the later questions.

What do you think? Is this worth pursuing ? Would you use it?

I've wanted to suggest something like that but I wasn't really sure if that was something well supported by ResourceLoader among other systems. It kind of throws LESS out the window in some ways? Ignoring any technical hurdles which are apparently potentially surmountable, I'm not totally certain it's a big win to do that in contrast e.g. to Common.css being loaded everywhere/Mobile.css being render blocking, both of which move away from technical debt rather than adding some random batch of configuration specifically for "hacks" (some of which are actual hacks and some are not).

Of the ones above that could definitely see a separate config value being useful, I'd say the ambox feature that Minerva adds, but I guess en.wp could definitely just pull that onwiki (not necessarily an answer for everyone) since we have some precedent of a different form locally. The rest all kind of come down to TemplateStyles or removal for the most part in my mind.

I think en.wp is maybe not the best wiki to ask about whether this approach would be useful? As your commit note says, we're mostly able to take care of ourselves and just haven't . Something true of most of the big wikis I think. So then you're back to medium/small wikis who may actually simply need things or not need things (i.e., just the one config that already exists). Kind of just generally conflicted all around about the idea. :)

I think this exercise would be a good opportunity to revise each of these as you've outlined in detail.

I think the exercise of splitting this content up would be one way to sort out what 'needs' to be here, and clearly WMF has left themselves a note previously about the utility of hacks.less (n.b. I see another block was added for T358164 and maybe T358797 not far behind? probably [bgcolor] and [style=bg] feel like they should end up not in hacks.less). But, I also think we can figure out which are valuable before spending a ton of time embarking down that road. (With, as the probably notable exception, deciding which content in the ambox block is 'style' and which is 'supports expansion' -- maybe an ambox improvement there either way for the latter that could use mw-collapsible directly instead.)

Maybe send a Tech News note to see who else would like to comment, and maybe we can all get to a decision about general utility together since I'm sure at least the other large wikis have tech contributors who can give a comment or two and/or whether they can help. Maybe there's a story there where something like that hackathon for TemplateStyles task get worked, or the task you put up about large gadget load on certain wikis.

Icelandic Wikipedia here. We mostly use English Wikipedia styles, with a handful of styles from spanish wikipedia.

Not sure where this hacks.less file is used, but I only see only one change for is.wp:

.collapsible  td

should be

.mw-collapsible td, .collapsible td

as we mostly use mw-collapsible. I would think that also allows en.wikipedia to migrate to mw.collapsible, so maybe they should be the same regardless?
There is no point for me to add to templates "collapsible" alongside "mw-collapsible", as en.wikipedia is migrating away from it anyway.

I am also seeing some outdated code: .content and .thumb.

Not sure where this hacks.less file is used,

I've added a description that explains what it is. My long comment excerpts the code in it, but I've also linked it in the description.

one change for is.wp: [snip]

I think mw-collapsible already has the styles for whatever this line is doing, but I haven't checked. (If it were a problem, I think we would already have heard about it somewhere?)

Okay I think I've encapsulated the work here in T360388 and T360387 (subtasks of T358071). Is this ticket good to be resolved? Is there anything I did not cover?

"Is this done?":

  1. As I said above, I don't think what is now at https://github.com/wikimedia/mediawiki-extensions-WikimediaMessages/blob/master/modules/ext.wikimediamessages.styles/navbox.less needs to exist anymore.: "I don't really get what this is doing here in this day and age. Minerva the skin shouldn't care, and these classes furthermore get removed by MobileFrontend." Is there another reason it still exists? We should document that if so. I can see we're using the class name for responsive skins now, so that could imply the effort I think I've seen for Vector 22, but then this is going to have negative impacts for users if/when Vector 22 gets skin--responsive at the resolution this file currently turns navboxes off at. I think leaving local wikis to figure out going forward how to deal with navboxes on responsive skins in general (with a pointer to the relevant tasks that also exist that are open about some options, including the media query to turn them off below a certain resolution).
  2. I think https://github.com/wikimedia/mediawiki-extensions-WikimediaMessages/blob/master/modules/ext.wikimediamessages.styles/ambox.less still needs work to split into two items: one supporting the "gadget-like" popup Javascript etc., and one with the other "let's make design decisions that need to be overridden onwiki". I can file this as a separate task since that looks like non-trivial nitpicky work.

"Is this done?":

  1. As I said above, I don't think what is now at https://github.com/wikimedia/mediawiki-extensions-WikimediaMessages/blob/master/modules/ext.wikimediamessages.styles/navbox.less needs to exist anymore.: "I don't really get what this is doing here in this day and age. Minerva the skin shouldn't care, and these classes furthermore get removed by MobileFrontend." Is there another reason it still exists? We should document that if so. I can see we're using the class name for responsive skins now, so that could imply the effort I think I've seen for Vector 22, but then this is going to have negative impacts for users if/when Vector 22 gets skin--responsive at the resolution this file currently turns navboxes off at. I think leaving local wikis to figure out going forward how to deal with navboxes on responsive skins in general (with a pointer to the relevant tasks that also exist that are open about some options, including the media query to turn them off below a certain resolution).

Minerva desktop doesn't remove HTML so without this resizing the Minerva skin will break display. Soon, my hope is Vector 2022 will become more responsive, and this rule may become important there to avoid horizontal scrolling. I agree it could do with some documentation. This may become redundant after T330527 so I promise to check in after that's fixed to see if we can remove it.

  1. I think https://github.com/wikimedia/mediawiki-extensions-WikimediaMessages/blob/master/modules/ext.wikimediamessages.styles/ambox.less still needs work to split into two items: one supporting the "gadget-like" popup Javascript etc., and one with the other "let's make design decisions that need to be overridden onwiki". I can file this as a separate task since that looks like non-trivial nitpicky work.

Yeh the ambox stuff is a bit messy as it encompasses JS and CSS and I agree this needs more thought and work. There is an argument to be made the JS should also be moved to WikimediaMessages. Thanks in advance for the ticket.

Minerva desktop doesn't remove HTML so without this resizing the Minerva skin will break display.

I don't understand this comment. The CSS in question removes the display below a certain resolution, but that resolution is irrelevant to desktop users. That's ignoring that the skin shouldn't care (it's a hack).

Vector 2022 will become more responsive, and this rule may become important there to avoid horizontal scrolling

As I said, you will have conniptions if this CSS goes live for Vector 22 as-is. We've also already seen prior complaint for the few users who use Minerva either on desktop website on mobile, or on desktop website on desktop.

Actually, now that I'm thinking about T330527, .navbox is a wrapper div these days (at least on en.wp), so we could just do overflow-x onwiki and then it would be resolution friendlier .........???? Even more reason not to have this CSS in place at all.

Yeh the ambox stuff is a bit messy as it encompasses JS and CSS and I agree this needs more thought and work. There is an argument to be made the JS should also be moved to WikimediaMessages. Thanks in advance for the ticket.

T365420: Split ambox.less

Change #1038900 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/WikimediaMessages@master] Drop navbox styles from WikimediaMessages

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

Jdlrobson claimed this task.
Jdlrobson updated the task description. (Show Details)

Sorry for the confusion. Regarding navboxes, now T330527 is resolved I think we can safely remove it.

We may need to add noresize to navbox on wiki, but we can remove the code in WikimediaMessages.

We've also scoped amboxes to Minerva.

Change #1038900 merged by jenkins-bot:

[mediawiki/extensions/WikimediaMessages@master] Drop navbox styles from WikimediaMessages

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