r105870 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r105869‎ | r105870 | r105871 >
Date:12:47, 12 December 2011
Author:amire80
Status:ok (Comments)
Tags:
Comment:
Followup to r105854. Made directionality inline rather than block-level, because text alignment should be consistent with the rest of the site. Added support for Monobook and Modern skins.
Modified paths:
  • /trunk/phase3/skins/Modern.php (modified) (history)
  • /trunk/phase3/skins/MonoBook.php (modified) (history)
  • /trunk/phase3/skins/Vector.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/MonoBook.php
——@@ -69,7 +69,7 @@
7070 <a id="top"></a>
7171 <?php if($this->data['sitenotice']) { ?><div id="siteNotice"><?php $this->html('sitenotice') ?></div><?php } ?>
7272
73 - <h1 id="firstHeading" class="firstHeading"><?php $this->html('title') ?></h1>
 73+ <h1 id="firstHeading" class="firstHeading"><span dir="auto"><?php $this->html('title') ?></span></h1>
7474 <div id="bodyContent" class="mw-body">
7575 <div id="siteSub"><?php $this->msg('tagline') ?></div>
7676 <div id="contentSub"<?php $this->html('userlangattributes') ?>><?php $this->html('subtitle') ?></div>
Index: trunk/phase3/skins/Modern.php
——@@ -52,7 +52,7 @@
5353 ?>
5454
5555 <!-- heading -->
56 - <div id="mw_header"><h1 id="firstHeading"><?php $this->html('title') ?></h1></div>
 56+ <div id="mw_header"><h1 id="firstHeading"><span dir="auto"><?php $this->html('title') ?></span></h1></div>
5757
5858 <div id="mw_main">
5959 <div id="mw_contentwrapper">
Index: trunk/phase3/skins/Vector.php
——@@ -134,7 +134,9 @@
135135 <!-- /sitenotice -->
136136 <?php endif; ?>
137137 <!-- firstHeading -->
138 - <h1 id="firstHeading" class="firstHeading" dir="auto"><?php $this->html( 'title' ) ?></h1>
 138+ <h1 id="firstHeading" class="firstHeading">
 139+ <span dir="auto"><?php $this->html( 'title' ) ?></span>
 140+ </h1>
139141 <!-- /firstHeading -->
140142 <!-- bodyContent -->
141143 <div id="bodyContent">

Follow-up revisions

RevisionCommit summaryAuthorDate
r105899Follow-up to r105870: dir="auto" on page title also for legacy skins (cologne...robin16:23, 12 December 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r105854Fixes Bug 32403. Will only work on browsers that support dir="auto". Won't br...amire8006:31, 12 December 2011

Comments

#Comment by Nikerabbit (talk | contribs)   14:41, 12 December 2011

What's the difference between block and inline direction?

#Comment by SPQRobin (talk | contribs)   15:02, 12 December 2011

When dir="auto" is on the h1 element, the whole title is at either the left or the right. Having a span tag leaves the title at the side of the user language direction, but makes neutral-direction characters (like exclamation marks) still display correctly.

So I think this commit is good, although the remaining skins need dir="auto" as well.

#Comment by Nikerabbit (talk | contribs)   15:04, 12 December 2011

Thanks for the explanation.

#Comment by Fomafix (talk | contribs)   08:12, 2 May 2012

The line wrap in Vector.php line 139 generates a trailing space in the clipboard when selecting the whole line with triple click in title. The other skins doesn't have this problem because they haven't a line wrap.

Examples:

#Comment by Amire80 (talk | contribs)   12:36, 2 May 2012

Thanks for the comment. Should be addressed at https://gerrit.wikimedia.org/r/#change,6382 .

Status & tagging log