Page MenuHomePhabricator

HtmlFormatter incorrectly removes partial matches for elements with hyphens
Open, LowPublic

Description

I’m not sure whether this is a bug or a feature, but in any case it is a behaviour that (to my knowledge) is not documented and therefore not expectable. If I assign the CSS class nomobile to an element, either via TemplateStyles or via inline styles, it will not only be hidden in the mobile version of a wiki page, but even deleted entirely from the HTML source. Fine. But apparently this happens also if the class is called something-nomobile, as it happened to me recently when using TemplateStyles (per convention on deWP, I use a unique identifier before all my individual class names, in this case charts-, which is supposed to prevent such class conflicts)! According to my tests, somethingnomobile or nomobilesomething don’t cause any trouble, only the version with the hyphen is apparently equalised with the actual nomobile class. I feel like class names that only contain nomobile should never trigger this behaviour or is there any particular reason for it?

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 25 2019, 12:20 PM
Jdlrobson renamed this task from CSS nomobile class conflict to HtmlFormatter incorrectly removes partial matches for elements with hypens.EditedAug 26 2019, 3:18 PM
Jdlrobson removed a project: MobileFrontend.
Jdlrobson added a subscriber: Jdlrobson.

Adding a class nomobile will delete content from the mobile domain and is used and documented in a variety of places.

I was surprised to hear that classes that match nomobile and tried a few that match and can see that the hypen does cause problems so this is a bug that will probably have to be worked around for the time being by not using -

The issue is in https://github.com/wikimedia/html-formatter/blob/master/src/HtmlFormatter.php#L183 which is bundled in core via composer. I'm not sure how to tag that correctly... but this is indeed a bug. @Aklapper do you know which tags would be appropriate?

Maybe this is a good motivation for adopting Remex?

Alright, I have now added a warning to the German WP project pages. In my case I have simply renamed the class in hidemobile for the moment.

Anomie added a subscriber: Anomie.

The issue is in https://github.com/wikimedia/html-formatter/blob/master/src/HtmlFormatter.php#L183 which is bundled in core via composer. I'm not sure how to tag that correctly... but this is indeed a bug. @Aklapper do you know which tags would be appropriate?

It doesn't look like we have a tag for the HtmlFormatter library, which would be the correct tag for it.

This analysis seems to be correct. The linked line should be something like

$elements = $xpath->query( '//*[contains(concat(" ", normalize-space(@class), " "), " ' . $classToRemove . ' ")]' );

Other XPath code in that library should probably also be checked for similar issues.

Dinoguy1000 renamed this task from HtmlFormatter incorrectly removes partial matches for elements with hypens to HtmlFormatter incorrectly removes partial matches for elements with hyphens.Aug 28 2019, 9:37 AM
WDoranWMF triaged this task as Low priority.Sep 11 2019, 2:34 PM