Page MenuHomePhabricator

Enforced   breaks inline CSS with !important
Closed, ResolvedPublic

Description

It's sometimes needed to enter !important in inline CSS styles. But if you try to do it, for example, with this code:

<div style="width:50% !important">

the generated wikitext will contain

<div style="width:50%&#160;!important">

This &#160; breaks style and some functionality.


Version: 1.12.x
Severity: normal
URL: http://ru.wikipedia.org/wiki/Template_talk:Ambox
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=3158

Details

Reference
bz11874

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:00 PM
bzimport set Reference to bz11874.
bzimport added a subscriber: Unknown Object (MLST).

nicdumz wrote:

proposed patch

It came from the preg_replace operated in parse() on the HTML :

'/(.) (?=\\?|:|;|!|%|\\302\\273)/' => '\\1&nbsp;\\2'

One-line patch, tested on a local wiki : it's adding a preg_replace rule :

'/&nbsp;(!important)/' => ' \\1',

To convert "!important" magic word back to its previous valid syntax. Yes, in that particular case, it converts " !important" to "&nbsp;!important", and then back to " !important", but I don't see a light way to improve the first regexp to avoid converting the space before " !important".

Cheers,

Nicolas.

Attached:

Looks good! Applied with parser test cases in r32375

Slight modification in r32377 for the case where space is present between ! and important

ayg wrote:

Why does parse() mangle spaces in style attributes to begin with? Surely that's got to cause other problems? As for the fix in r32377, won't "width:50% ! important" become "width:50%&nbsp;!&nbsp;important" and so evade the fix?

ayg wrote:

(In reply to comment #4)

As for the fix in r32377, won't "width:50%
! important" become "width:50%&nbsp;!&nbsp;important" and so evade the fix?

No, of course not, never mind me. That only applies to spaces *before* exclamation points. Still seems like the best way to fix this would be to avoid HTML attributes, or at least some of them (like style attributes).

Presumably so that title, alt, etc texts will also wrap properly.

Perhaps such attributes could be whitelisted, which would solve this in a more long-term way.

ayg wrote:

Yes, some attributes should have this, but not in general. I suspect that the replace could potentially even affect class declarations, although they'd be fairly funny class declarations. For style, the only things that should be replaced by this are quoted strings for properties like content.

Wouldn't similar issues to the one fixed occur with styles declared like "width : 50%"? That must be fairly common.

ui2t5v002 wrote:

This does screw up things like style="border : solid;" It should not apply inside HTML tags by default.

Change 441410 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] WIP: Don't armor french spaces before punctuation followed by word characters

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