r111861 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r111860‎ | r111861 | r111862 >
Date:13:06, 19 February 2012
Author:hartman
Status:resolved (Comments)
Tags:
Comment:
Bug 27619: Remove the option to show broken links as: link?

Being bold here, but otherwise it will still be there in 2016
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.20 (modified) (history)
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/Preferences.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderUserCSSPrefsModule.php (modified) (history)
  • /trunk/phase3/skins/Simple.php (modified) (history)
  • /trunk/phase3/skins/nostalgia/screen.css (modified) (history)
  • /trunk/phase3/skins/simple/main.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/Simple.php
@@ -36,13 +36,6 @@
3737 if ( $this->getUser()->getOption( 'underline' ) < 2 ) {
3838 $underline = "text-decoration: " . $this->getUser()->getOption( 'underline' ) ? 'underline !important' : 'none' . ";";
3939 }
40 -
41 - /* Also inherits from resourceloader */
42 - if( !$this->getUser()->getOption( 'highlightbroken' ) ) {
43 - $rules[] = "a.new, a.stub { color: inherit; text-decoration: inherit;}";
44 - $rules[] = "a.new:after { color: #CC2200; $underline;}";
45 - $rules[] = "a.stub:after { $underline; }";
46 - }
4740 $style = implode( "\n", $rules );
4841 $out->addInlineStyle( $style, 'flip' );
4942
Index: trunk/phase3/skins/nostalgia/screen.css
@@ -14,6 +14,10 @@
1515 padding: 2px;
1616 }
1717
 18+a.new {
 19+ color: #ba0000;
 20+}
 21+
1822 a.interwiki,
1923 a.external {
2024 color: #3366BB;
Index: trunk/phase3/skins/simple/main.css
@@ -345,6 +345,7 @@
346346 }
347347 a.new,
348348 #p-personal a.new {
 349+ color: #BA0000;
349350 text-decoration: line-through;
350351 }
351352 li.new {
Index: trunk/phase3/includes/resourceloader/ResourceLoaderUserCSSPrefsModule.php
@@ -92,13 +92,6 @@
9393 $rules[] = 'a:lang(ar), a:lang(ckb), a:lang(fa),a:lang(kk-arab), ' .
9494 'a:lang(mzn), a:lang(ps), a:lang(ur) { text-decoration: none; }';
9595 }
96 - if ( $options['highlightbroken'] ) {
97 - $rules[] = "a.new, #quickbar a.new { color: #ba0000; }\n";
98 - } else {
99 - $rules[] = "a.new, #quickbar a.new, a.stub, #quickbar a.stub { color: inherit; }";
100 - $rules[] = "a.new:after, #quickbar a.new:after { content: '?'; color: #ba0000; }";
101 - $rules[] = "a.stub:after, #quickbar a.stub:after { content: '!'; color: #772233; }";
102 - }
10396 if ( $options['justify'] ) {
10497 $rules[] = "#article, #bodyContent, #mw_content { text-align: justify; }\n";
10598 }
Index: trunk/phase3/includes/DefaultSettings.php
@@ -3231,7 +3231,6 @@
32323232 'gender' => 'unknown',
32333233 'hideminor' => 0,
32343234 'hidepatrolled' => 0,
3235 - 'highlightbroken' => 1,
32363235 'imagesize' => 2,
32373236 'justify' => 0,
32383237 'math' => 1,
Index: trunk/phase3/includes/Preferences.php
@@ -638,11 +638,6 @@
639639 );
640640
641641 if ( $wgAllowUserCssPrefs ) {
642 - $defaultPreferences['highlightbroken'] = array(
643 - 'type' => 'toggle',
644 - 'section' => 'rendering/advancedrendering',
645 - 'label' => $context->msg( 'tog-highlightbroken' )->text(), // Raw HTML
646 - );
647642 $defaultPreferences['showtoc'] = array(
648643 'type' => 'toggle',
649644 'section' => 'rendering/advancedrendering',
Index: trunk/phase3/RELEASE-NOTES-1.20
@@ -20,6 +20,7 @@
2121 * (bug 31417) New ID mw-content-text around the actual page text, without categories,
2222 contentSub, ... The same div often also contains the class mw-content-ltr/rtl.
2323 * (bug 34475) Add support for IP/CIDR notation to tablesorter
 24+* (bug 27619) Remove preference option to display broken links as link?
2425
2526 === Bug fixes in 1.20 ===
2627 * (bug 30245) Use the correct way to construct a log page title.

Follow-up revisions

RevisionCommit summaryAuthorDate
r111862Followup to r111861. Remove messages for highlightbroken because they are no ...hartman13:14, 19 February 2012
r112345follow-up to r111861, fixing classic skinkaldari20:03, 24 February 2012

Comments

#Comment by Siebrand (talk | contribs)   13:09, 19 February 2012

You also have to Remove the entry from messages.inc and MessagesEn.php

#Comment by Nikerabbit (talk | contribs)   07:08, 20 February 2012

Beware the wrath of text browser users.

#Comment by TheDJ (talk | contribs)   11:08, 20 February 2012

You mean the textbrowser users who already didn't get a different layout, because their browsers don't support css and especially the .content selector that was used to implement this ? Like the commenter in the bug says, the option is useless for text browsers unless implemented in pure html output. So I think it will be ok from that side.

#Comment by Nikerabbit (talk | contribs)   11:13, 20 February 2012

Nice tactic! First break something, and then say nobody uses it so it can be removed.

#Comment by GWicke (talk | contribs)   11:23, 20 February 2012

There are likely bigger issues for text browsers than missing link target existence indication. Removal of preference-dependent HTML from the content is important for better caching, and should really override this minor inconvenience for the very few. At least following the link will be very fast with a text browser, so you quickly see if the article exists or not ;)

#Comment by Saper (talk | contribs)   10:00, 29 February 2012

There are no other issues. I am frequently using w3m for editing, especially when editing large amount of articles (and because having external editor like vim fits nicely with this browser), and lack of "?" because it was broken by doing it in CSS literally DRIVES ME MAD.

I have to check all links I have added to verify I haven't made a mistake.

I understand there are caching issues w/parser and what not to get it right, but maybe small community using this option does not make it an operational problem really?

#Comment by Kaldari (talk | contribs)   19:41, 24 February 2012

Look like the Classic and MySkin skins need to be updated as well to get redlinks.

#Comment by Kaldari (talk | contribs)   19:56, 24 February 2012

Nevermind about MySkin, that skin apparently has no CSS on purpose.

#Comment by Kaldari (talk | contribs)   21:57, 24 February 2012

I fixed the classic skin in r112345.

Status & tagging log