Page MenuHomePhabricator

Kill $wgCleanupPresentationalAttributes from MediaWiki core
Closed, ResolvedPublic

Description

Splitting this from bug 40329, $wgCleanupPresentationalAttributes should be killed from MediaWiki core. It was introduced in r94465 to try to magically transform particular deprecated and invalid attributes into their CSS equivalents.

I don't believe these types of magical transformations are a good idea and the introduction of this code appears to have caused more problems than it has solved. I strongly urge that this global configuration variable and its related code be removed before the release of 1.20.


Version: unspecified
Severity: blocker
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=39614
https://bugzilla.wikimedia.org/show_bug.cgi?id=66413

Details

Reference
bz40632

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:50 AM
bzimport set Reference to bz40632.

mr.heat wrote:

I strongly agree. I said everything I have to say about this ugly hack in bug 40329.

But it won't happen in time for 1.20 tarball, so bumping to future.

I don't see how this is accessibility related. Please don't pollute that keyword.

Inline stylesheets are key accessibility problem.

Accessibility is supposed to be for things that affect the ability for blind users, deaf users, and users with motor-disabilities to use the site. I don't see how changing style="text-align: center;" back to align="center" has anything to do with that.

Even if you try to expand that to stuff important to a non-disabled user's ability to access the site this doesn't have anything to do with that either. The worst case scenario here is that some center or right aligned content will be left aligned. That has nothing to do with site accessibility of any kind.

And even if inline stylesheets are a problem this has nothing to do with that either. Inline deprecated presentational html attributes are no better than inline style="" attributes. This bug doesn't make anything better in that area.

I don't see this tagging this sas anything but pollution to that keyword taking away attention from bugs that actually get in the way of the ability for users with disabilities to access our wikis.

content hidden as private in Bugzilla

Nobody is talking about inline styles. This is technical representation only.
This is not related to accessibility.

(In reply to comment #3)

But it won't happen in time for 1.20 tarball, so bumping to future.

I'd like to beg for this to get back on the 1.20 tarball. We introduced a bad
and destructive "feature" in 1.20-git, and we need to get rid of it before
it goes out in any official release.

(In reply to comment #8)

Nobody is talking about inline styles. This is technical representation only.
This is not related to accessibility.

(In reply to comment #3)

But it won't happen in time for 1.20 tarball, so bumping to future.

I'd like to beg for this to get back on the 1.20 tarball. We introduced a bad
and destructive "feature" in 1.20-git, and we need to get rid of it before
it goes out in any official release.

Instead of trying to get a full removal done in time you could just flip the default in the 1.20 branch so you'll get the same effect out of a release.

Though I may have to burst your bubble a bit... This feature was introduced in 1.19 and it's already live in a stable release. Not just that but that included the <table style="text-align: right;"> bug. So it wasn't an issue introduced in 1.20, it's already live and 1.20 makes it less buggy.

(In reply to comment #8)

Nobody is talking about inline styles. This is technical representation only.
This is not related to accessibility.

The feature converts old presentational attributes to inline stylesheets.

(In reply to comment #10)

(In reply to comment #8)

Nobody is talking about inline styles. This is technical representation only.
This is not related to accessibility.

The feature converts old presentational attributes to inline stylesheets.

Neither of which are related to content, only style.

Accessibility related applications that still don't understand CSS, can't be taken seriously. And even if someone would disagree with that statement, its pointless. The vast majority of layout is already CSS-only (and has been for over almost a decade). So for whatever accessibility related software that doesn't understand CSS, a couple of "align=center" and "valign=top" attributes here and there are the least of its worries.

[15:06:47] <Ulfr> Reedy: Can you comment on the bug on my behalf? Something simple like 'Very yes.'
[15:06:56] <Ulfr> That bloody variable completely tanked my weekend
[15:07:41] <Ulfr> Well, that and CentOS being older than I realized :P

Blocking 1.20 instead of the other bugs related to this.

(In reply to comment #11)

(In reply to comment #10)

(In reply to comment #8)

Nobody is talking about inline styles. This is technical representation only.
This is not related to accessibility.

The feature converts old presentational attributes to inline stylesheets.

Neither of which are related to content, only style.

Accessibility related applications that still don't understand CSS, can't be
taken seriously. And even if someone would disagree with that statement, its
pointless. The vast majority of layout is already CSS-only (and has been for
over almost a decade). So for whatever accessibility related software that
doesn't understand CSS, a couple of "align=center" and "valign=top" attributes
here and there are the least of its worries.

Nobody was talking about any software not understanding CSS, but about inline styles. So again: *inline styles* are key accessibility problem.

Besides, it has been proved, that this feature does in certain cases wrong conversions, so in certain cases the mispositioning of elements causes overlapping or disappearing of content which obviously is an accessibility issue even for people with good eyes.

(In reply to comment #14)

Nobody was talking about any software not understanding CSS, but about inline
styles. So again: *inline styles* are key accessibility problem.

You can keep saying that, but it doesn't explain anything. How are inline styles (not CSS rules in general) related to accessibility?

(In reply to comment #14)

Besides, it has been proved, that this feature does in certain cases wrong
conversions.

Most certainly, which is why it should be removed.

(In reply to comment #14)

Besides, it has been proved, that this feature does in certain cases wrong
conversions, so in certain cases the mispositioning of elements causes
overlapping or disappearing of content which obviously is an accessibility
issue even for people with good eyes.

The only current bugs that have been shown are with block elements that are supposed to be left/right/center aligned not being aligned. There's no disappearance, etc... involved and no reason to expect that presentational html -> inline css conversion would create any issue like that.

So this is not an accessibility issue. Not to mention even that does not fit under the term accessibility. That would just be a general bug.

So why is this deferred to a later release? Due to nobody working on it or due to it not being important enough? I assume there are people assigning engineers to important things?

Just ran into another odd bug caused by this. Confirmed it worked with this removed.

Removed in Change-Id: I4e86305520a3b22ef88381caab55d24abac932e3.

(Citing comment #17: 2012-10-13)

So why is this deferred to a later release?

..

(Citing comment #18: 2012-11-01)

Remove, pending review: I4e86305520a3b22ef88381caab55d24abac932e3.

..

So, it went out in the 1.20.0 release. An unstable sanitizer that has proven to destroy random existing pages in unexpected ways will now be widespread.

(In reply to comment #19)

So, it went out in the 1.20.0 release. An unstable sanitizer that has proven to
destroy random existing pages in unexpected ways will now be widespread.

It's already been in there since 1.19. 1.20 actually fixes one of the issues in what went out in 1.19.

That said, it probably would've been easier to get 1.20 to just turn it off instead of removing it entirely as I mentioned.

Landed in master and merged to REL1_20.

  • Bug 41018 has been marked as a duplicate of this bug. ***