From: C. Scott Ananian <cscott@cscott.net>
Date: Fri, 8 Jun 2018 16:18:05 +0000 (-0400)
Subject: Deprecate Language::markNoConversion, which confuses readers
X-Git-Tag: 1.34.0-rc.0~5080
X-Git-Url: http://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=7de2c566dd0fb4d0d994867094a00899df107420

Deprecate Language::markNoConversion, which confuses readers

Language::markNoConversion is used only within Parser.php and differs
from LanguageConverter::markNoConversion in that, contrary to its name
and its namesake, it only protects *things which look like URLs* from
language conversion.

This wasted several days of my time before I realized what was going on.
It's needless; just hoist the "looks like a URL" special casing inline
to the single place where that functionality is used.  (And I wonder
if the "looks like a URL" case is actually needed at all any more,
since most of those cases are probably free external links, which
go through a different code path, not bracketed external links.)

This is a clean-up to the clean-up that liangent performed in 2012
with e01adbfc0bd9f39adffc1f955ccc711e73818194.

Change-Id: I80479600f34170651732b032e8881855aa1204d8
---

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index aa671d2984..5dbf873c86 100644
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -207,6 +207,10 @@ because of Phabricator reports.
   generate cryptographically secure random byte sequences.
 * Parser::getConverterLanguage() is deprecated.  Use ::getTargetLanguage()
   instead.
+* Language::markNoConversion() is deprecated.  It confused readers because
+  it had unexpected behavior (only marking text if it looked like a URL)
+  and was only used in a single place in the code.  Use
+  LanguageConverter::markNoConversion() instead.
 
 === Other changes in 1.32 ===
 * …
diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index d3eff8a12c..38bc95e802 100644
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -1606,7 +1606,7 @@ class Parser {
 		if ( $text === false ) {
 			# Not an image, make a link
 			$text = Linker::makeExternalLink( $url,
-				$this->getTargetLanguage()->markNoConversion( $url, true ),
+				$this->getTargetLanguage()->getConverter()->markNoConversion( $url ),
 				true, 'free',
 				$this->getExternalLinkAttribs( $url ), $this->mTitle );
 			# Register it in the output object...
@@ -1895,7 +1895,10 @@ class Parser {
 				list( $dtrail, $trail ) = Linker::splitTrail( $trail );
 			}
 
-			$text = $this->getTargetLanguage()->markNoConversion( $text );
+			// Excluding protocol-relative URLs may avoid many false positives.
+			if ( preg_match( '/^(?:' . wfUrlProtocolsWithoutProtRel() . ')/', $text ) ) {
+				$text = $this->getTargetLanguage()->getConverter()->markNoConversion( $text );
+			}
 
 			$url = Sanitizer::cleanUrl( $url );
 
diff --git a/languages/Language.php b/languages/Language.php
index bffed7f224..d762a5752c 100644
--- a/languages/Language.php
+++ b/languages/Language.php
@@ -4321,13 +4321,18 @@ class Language {
 	 * the "raw" tag (-{R| }-) to prevent conversion.
 	 *
 	 * This function is called "markNoConversion" for historical
-	 * reasons.
+	 * reasons *BUT DIFFERS SIGNIFICANTLY* from
+	 * LanguageConverter::markNoConversion(), with which it is easily
+	 * confused.
 	 *
 	 * @param string $text Text to be used for external link
 	 * @param bool $noParse Wrap it without confirming it's a real URL first
 	 * @return string The tagged text
+	 * @deprecated since 1.32, use LanguageConverter::markNoConversion()
+	 *  instead.
 	 */
 	public function markNoConversion( $text, $noParse = false ) {
+		wfDeprecated( __METHOD__, '1.32' );
 		// Excluding protocal-relative URLs may avoid many false positives.
 		if ( $noParse || preg_match( '/^(?:' . wfUrlProtocolsWithoutProtRel() . ')/', $text ) ) {
 			return $this->mConverter->markNoConversion( $text );