Last modified: 2014-11-17 10:34:42 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T42632, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 40632 - Kill $wgCleanupPresentationalAttributes from MediaWiki core
Kill $wgCleanupPresentationalAttributes from MediaWiki core
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
unspecified
All All
: Normal blocker with 3 votes (vote)
: 1.20.x release
Assigned To: Krinkle
:
: 41018 (view as bug list)
Depends on:
Blocks: 40329
  Show dependency treegraph
 
Reported: 2012-09-30 01:59 UTC by MZMcBride
Modified: 2014-11-17 10:34 UTC (History)
9 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description MZMcBride 2012-09-30 01:59:54 UTC
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.
Comment 1 TMg 2012-09-30 13:04:17 UTC
I strongly agree. I said everything I have to say about this ugly hack in bug 40329.
Comment 2 Danny B. 2012-09-30 13:06:27 UTC
+1
Comment 3 Mark A. Hershberger 2012-09-30 16:26:08 UTC
But it won't happen in time for 1.20 tarball, so bumping to future.
Comment 4 Daniel Friesen 2012-09-30 16:30:37 UTC
I don't see how this is accessibility related. Please don't pollute that keyword.
Comment 5 Danny B. 2012-09-30 16:36:54 UTC
Inline stylesheets are key accessibility problem.
Comment 6 Daniel Friesen 2012-09-30 17:04:07 UTC
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.
Comment 8 Krinkle 2012-09-30 17:28:08 UTC
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.
Comment 9 Daniel Friesen 2012-09-30 17:32:32 UTC
(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.
Comment 10 Danny B. 2012-09-30 17:37:48 UTC
(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.
Comment 11 Krinkle 2012-09-30 22:16:31 UTC
(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.
Comment 12 Sam Reed (reedy) 2012-10-01 14:18:06 UTC
[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
Comment 13 Krinkle 2012-10-01 21:38:40 UTC
Blocking 1.20 instead of the other bugs related to this.
Comment 14 Danny B. 2012-10-03 16:57:43 UTC
(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.
Comment 15 Krinkle 2012-10-03 17:05:58 UTC
(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.
Comment 16 Daniel Friesen 2012-10-03 17:09:29 UTC
(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.
Comment 17 Krinkle 2012-10-13 22:07:53 UTC
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?
Comment 18 Krinkle 2012-11-01 18:11:42 UTC
Just ran into another odd bug caused by this. Confirmed it worked with this removed.

Removed in Change-Id: I4e86305520a3b22ef88381caab55d24abac932e3.
Comment 19 Krinkle 2012-11-19 16:10:36 UTC
(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.
Comment 20 Daniel Friesen 2012-11-19 23:41:49 UTC
(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.
Comment 21 Krinkle 2012-11-20 19:10:53 UTC
Landed in master and merged to REL1_20.
Comment 22 Kevin Israel (PleaseStand) 2012-11-30 00:41:53 UTC
*** Bug 41018 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links