r69649 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r69648‎ | r69649 | r69650 >
Date:23:47, 20 July 2010
Author:tparscal
Status:ok (Comments)
Tags:brion 
Comment:
This improves on the CSS hack used in r69317 - which worked fine but in some crazy extreme hypothetical case could end up showing the text.
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/skins/vector/main-ltr.css (modified) (history)
  • /trunk/phase3/skins/vector/main-rtl.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/vector/main-ltr.css
——@@ -1111,8 +1111,10 @@
11121112 outline: none;
11131113 display: block;
11141114 width: 26px;
1115 - height: 3.1em;
1116 - text-indent: -9999px;
 1115+ /* This hides the text but shows the background image */
 1116+ padding-top: 3.1em;
 1117+ height: 0;
 1118+ overflow: hidden;
11171119 }
11181120 #ca-unwatch.icon a {
11191121 background-image: url(images/watch-icons.png?1);
Index: trunk/phase3/skins/vector/main-rtl.css
——@@ -1111,8 +1111,10 @@
11121112 outline: none;
11131113 display: block;
11141114 width: 26px;
1115 - height: 3.1em;
1116 - text-indent: -9999px;
 1115+ /* This hides the text but shows the background image */
 1116+ padding-top: 3.1em;
 1117+ height: 0;
 1118+ overflow: hidden;
11171119 }
11181120 #ca-unwatch.icon a {
11191121 background-image: url(images/watch-icons.png?1);
Index: trunk/phase3/includes/DefaultSettings.php
——@@ -1547,7 +1547,7 @@
15481548 * to ensure that client-side caches do not keep obsolete copies of global
15491549 * styles.
15501550 */
1551 -$wgStyleVersion = '298';
 1551+$wgStyleVersion = '299';
15521552
15531553 /**
15541554 * This will cache static pages for non-logged-in users to reduce

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r69317Epic bug fix. Changing the markup structure of Vectors tabs and related watch...adam22:31, 13 July 2010

Comments

#Comment by Platonides (talk | contribs)   13:56, 21 July 2010

I had also studied this alternative to r69649, but it presented problems with IE (additional hacks would be needed for it to work on its several versions).

Have you tried this in Internet Explorer?

#Comment by Trevor Parscal (WMF) (talk | contribs)   17:34, 26 July 2010

Yes, 6, 7 and 8. Which versions are you seeing issues?

#Comment by Platonides (talk | contribs)   17:53, 26 July 2010

With $wgVectorUseIconWatch = true; the star is "down", touching the baseline.

#Comment by Simetrical (talk | contribs)   21:50, 21 July 2010

Isn't the idea of using giant pixel offsets usually that screen readers will read the text, whereas if you do visibility: hidden or display: none, they won't?

#Comment by Catrope (talk | contribs)   21:53, 21 July 2010

Isn't that what he's doing? Note that overflow: hidden; means something very different from visibility: hidden;. AFAIK screen readers should still read this text.

#Comment by Simetrical (talk | contribs)   22:05, 21 July 2010

Bleh, I misread. Maybe this will work too, I don't know (which is why I didn't set fixme . . .). I only have any idea how screen readers work by hearsay. If this works too, fine by me.

#Comment by Platonides (talk | contribs)   21:55, 21 July 2010

This is screen reader friendly.

#Comment by Krinkle (talk | contribs)   00:42, 8 February 2011

Don't have the time to test this in IE browsers right now, so I'm not going to mark it as 'ok'. But something that came to mind is this (old) article by Stuart Langridge (always keep these things in bookmarks for later reference! ;-) ).

The only catch back then was about IE5. But no concerns for IE6 that I know of.

#Comment by Brion VIBBER (talk | contribs)   02:09, 8 February 2011

Seems all fine, I've just done a test in IE 6, 7, and 8 to confirm. The only remaining alignment issues I could find were with the spinner, (bugzilla:27230) which isn't related to this (this is a vertical alignment thing, that was a horizontal alignment thing) and is resolved separately in r81679.

Marking this rev as OK.

#Comment by Krinkle (talk | contribs)   00:44, 8 February 2011

See also r81430

Status & tagging log