Page MenuHomePhabricator

User styles for .hlist clash with ones in core and should be scoped
Closed, DuplicatePublic

Description

In testwiki wmf.12 elements in pre-content heading-holder seems to be overlapping each other:

Screen Shot 2019-01-08 at 6.03.44 PM.png (679×1 px, 395 KB)

enwiki wmf.9 is displayed correctly:

Screen Shot 2019-01-08 at 6.03.35 PM.png (639×1 px, 372 KB)

Event Timeline

Jdlrobson added a subscriber: Jdlrobson.

Only appears to be an issue on Minerva desktop.

https://en.m.wikipedia.beta.wmflabs.org/wiki/Paris is rendering fine.

that code is disappearing next week so probably best to just wait.

I stand corrected, post-page issues deploy it looks worse:

Screen Shot 2019-01-08 at 10.00.20 PM.png (161×1 px, 22 KB)

There is something in JS that is loading as this UI regression arrives later. I'll debug tomorrow about what that could be.

The issue is due to the .hlist rule in https://test.wikipedia.org/wiki/MediaWiki:Common.css

.hlist dd:after, .hlist li:after

This is a on-wiki issue. Those rules should be scoped to #content.

Jdlrobson reopened this task as Open.EditedJan 9 2019, 5:18 PM
Jdlrobson edited projects, added MinervaNeue (Tracking); removed MinervaNeue (Desktop).

I guess this is valid, I'm just not sure who and how we handle this. It does impact desktop enwiki

Jdlrobson renamed this task from [testwiki regression wmf.12] MinervaNeue - pre-content heading-holder has overlapping elements to User styles for .hlist clash with ones in core and should be scoped.Jan 11 2019, 4:12 PM

Seeing this bug on mobile today -- Android/Samsung Galaxy/Chrome/logged out

image.png (2×1 px, 441 KB)

https://da.m.wikipedia.org/wiki/F%C3%A5r

I got dawiki fixed and checked all the biggest wikis and none of them have this problem. I don't think it's sustainable for us to hot-fix CSS rules provided by community members and we need to get better at outreach and helping those editors to work with us a la page issues. I'm not quite sure what the next step is with this task and what further analysis we want to do. This is a problem we live with lacking a long term solution.

We could post something to the talk pages of all Mediawiki:mobile.css with recommendations. The conundrum is that we only want to message talk pages where this run is in effect. A global Wikimedia search engine would be nice right about now.

This is an on-wiki issue.

Uhhhhhhhh pretty sure .hlist was introduced by the editor community first. That note and comment blaming the community is quite a bit obnoxious on the point.

I'm also not sure if dropping a #bodyContent on that is really all that great a response to this ticket, given the specificity of IDs generally. Isn't the correct fix to correct Mobile.css or the skin-specific CSS?

I don't think it's sustainable for us to hot-fix CSS rules provided by community members and we need to get better at outreach and helping those editors to work with us a la page issues.

Isn't the biggest issue here that you're reusing an originally editor-defined class in a way specific to the mobile site without having removed the editing community's definitions?

Or perhaps the issue is that Common.css should be deprecated in favor of the skin CSS pages?

.hlist is on the table for moving to TemplateStyles, but there are many thousands of pages where it is used "inline" or non-trivially (every invocation of hlist in navbox could be coded around, but there are still others) rather than through some sort of wrapper template.

Uhhhhhhhh pretty sure .hlist was introduced by the editor community first. That note and comment blaming the community is quite a bit obnoxious on the point.
Isn't the biggest issue here that you're reusing an originally editor-defined class in a way specific to the mobile site without having removed the editing community's definitions?

You are correct .hlist was introduced by the editor community. It was then added to core in T42062 (cc @He7d3r) and with that came the expectation that it could be used by skins eg. Minerva which was being built at the time. IT seems like that may have been done prematurely, see also the related T174399. I pushed specifically for TemplateStyles to solve this problem (T483).

In mobile we're trying to squeeze every possible byte out for performance reasons so we decided to use this module for horizontal lists that appeared elsewhere in Minerva. Mobile is our largest audience of traffic and we see a lot of users with poor connections, so I stand by that decision.

Moving the community styles to TemplateStyles would be awesome, but I understand the challenges involved in this.

'm also not sure if dropping a #bodyContent on that is really all that great a response to this ticket, given the specificity of IDs generally. Isn't the correct fix to correct Mobile.css or the skin-specific CSS?

The issue was reported for the desktop skin of Minerva. Mobile is fine so I don't really mind what you do here. From my perspective the problem is these styles are not Common to all skins. They are common to all skins except Minerva. The mobile skin is just a skin, so you should be able to load an override on MediaWiki:Minerva.css. I changed the MediaWiki:Common.css version since that seemed non-controversial to me, the most simple possible change and was apparently affecting users. I don't see why scoping these rules to #bodyContent is problematic. Can you elaborate on your concerns here?

That note and comment blaming the community is quite a bit obnoxious on the point.

I'm sorry you interpreted it this way. I was not blaming anyone, I was simply saying this is something that needs to be fixed socially like we are doing right now, and not in the codebase via the addition of technical debt hence an "on-wiki issue" as in we need to make some changes in the wiki.

Yeah, I'm probably just a bit grouchy today.

Is this recent report by an IP possibly related to the changes today?

In mobile we're trying to squeeze every possible byte out for performance reasons so we decided to use this module for horizontal lists that appeared elsewhere in Minerva. Mobile is our largest audience of traffic and we see a lot of users with poor connections, so I stand by that decision.

It would help if the styles didn't conflict. See Jack's comment on T174399#3736517. I'm not sure how much work on your team's part that would be.

I pushed specifically for TemplateStyles to solve this problem (T483).
Moving the community styles to TemplateStyles would be awesome, but I understand the challenges involved in this.

Maybe some work could be done there to move styles out of Common.css and elsewhere by CommTech or your team or? Hlist isn't the only thing--.infobox is in that category also.

Maybe this could be taken out of core then? We now have another template (Template:Cslist) which is an inline list with commas instead (for better or worse). The choice of list delimiter is a bit more a style point than e.g. .wikitable.

I'm also not sure if dropping a #bodyContent on that is really all that great a response to this ticket, given the specificity of IDs generally. Isn't the correct fix to correct Mobile.css or the skin-specific CSS?

From my perspective the problem is these styles are not Common to all skins. They are common to all skins except Minerva.

Was that a conscious choice at the beginning of mobile development...? Is that now a feature or a bug?

The mobile skin is just a skin, so you should be able to load an override on MediaWiki:Minerva.css. I changed the MediaWiki:Common.css version since that seemed non-controversial to me, the most simple possible change and was apparently affecting users. I don't see why scoping these rules to #bodyContent is problematic. Can you elaborate on your concerns here?

!important is bad for overriding things. It's really hard to override IDs also. I had to put in an edit request just the other day to lower the specificity in Common.css so that we could correct a TemplateStyles bug. I'd probably have been okay with .mw-parser-output... but if it's a skin specific issue, it should really be taken care of in the specific skin.

On an aside, not all the skins have common IDs (Modern; see above edit request. I think Isarra noticed that the other day onwiki somewhere.)

That note and comment blaming the community is quite a bit obnoxious on the point.

I'm sorry you interpreted it this way. I was not blaming anyone, I was simply saying this is something that needs to be fixed socially like we are doing right now, and not in the codebase via the addition of technical debt hence an "on-wiki issue" as in we need to make some changes in the wiki.

There probably needs to be a think or two about which styling/HTML classes should be core or assumed by core and which shouldn't be, especially now with TemplateStyles, but also with mobile, but also with all of every wiki having potentially different styling. T71550 is the relevant one there?

Maybe this could be taken out of core then?

Yes, possibly the best outcome here would be to move hlist styles into a template style and remove the hlist styles from core. The benefit of this is that editor hlist styles will be only loaded on the pages that need them (rather than every page) and Minerva can use some much more light weight styling for those inline lists.

Removing conflicts between the two is quite straight forward but would mean all pages would have duplicate rules for the common bits.

Was that a conscious choice at the beginning of mobile development...? Is that now a feature or a bug?

Before Minerva all skins were pretty much exactly the same with different CSS. Minerva's HTML is very different from the skins that preceded it and obviously Mediawiki:common predates that. TemplateStyles IMO makes this page redundant as all styles belong to specific templates other than UI tweaks which should be put in the appropriate skin css. Maybe the solution here is to slowly deprecate Mediawiki:common.css for skin specific templates e.g. Mediawiki:vector.css etc

The problem with mediawiki:common.css is the rules there probably apply to something like 20% of pages so much of the CSS in unused.

but if it's a skin specific issue, it should really be taken care of in the specific skin.

think using Mediawiki:Minerva.css in the mean time to override mediawiki:common rules is probably the best way forward. I don't know why I didn't do that in the first place so thanks for flagging it. You've also made me wonder if Minerva should also not load Mediawiki:Common.css to encourage migrating the essential styles.

Jdlrobson lowered the priority of this task from High to Low.Aug 1 2019, 8:40 PM

@Jdlrobson I am fairly sure these are the same issue. Hlist was fine until recently.

Unless you just up and decided to stop styles loading where appropriate, which would have been a Tech News kind of issue...

@Izno Hlist styles have never been fine on mobile. The hlist styles are a mess on Minerva and are a longstanding issue. We have not made any changes to them recently, but it doesn't surprise me these have broken. The problem is styles are shared between MediaWiki:Common.css, Minerva and templates with various problems and various gadgets also tweak them. As stated on T174399 the plan is to remove these styles from core and Minerva, but this can't happen until wikis are ready for us to do them. Note, MediaWiki:Mobile.css will eventually be removed as well so the only solution is to move these into template style rules.

I'm not sure what caused the main page issue you reported in T293232 but Minerva has never styled div.hlist which is in the HTML the main page produces and MediaWiki:Mobile.css does not work in the same way as MediaWiki:Common.css. In this particular case the main page needs to include that style like so https://en.wikipedia.org/w/index.php?title=Wikipedia%3AMain_Page%2Fstyles.css&type=revision&diff=1049785579&oldid=1004580458 - obviously having a shared stylesheet for all of this would be the most ideal but as far as I am concerned "hlist" should be considered an editor-maintained style from now. This goal was what prompted my question on https://en.wikipedia.org/wiki/Module_talk:List#Inlining_styles_via_TemplateStyles._Who_can_help? but I'm not sure what the status of that conversation is.

I'm going to merge this into T213239 as this is all the same issue.

but I'm not sure what the status of that conversation is.

I moved the discussion to Common.css and then responded to you, where I was hoping others would chime in. Please feel free to follow up there yourself.