r68358 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r68357‎ | r68358 | r68359 >
Date:14:08, 21 June 2010
Author:conrad
Status:reverted (Comments)
Tags:
Comment:
(bug 18431) Remove initial whitespace from fragment in Title::secureAndSplit()
Modified paths:
  • /trunk/phase3/includes/Title.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Title.php
——@@ -2563,7 +2563,7 @@
25642564 }
25652565 $fragment = strstr( $dbkey, '#' );
25662566 if ( false !== $fragment ) {
2567 - $this->setFragment( $fragment );
 2567+ $this->setFragment( preg_replace( '/^#_*/', '#', $fragment ) );
25682568 $dbkey = substr( $dbkey, 0, strlen( $dbkey ) - strlen( $fragment ) );
25692569 # remove whitespace again: prevents "Foo_bar_#"
25702570 # becoming "Foo_bar_"

Follow-up revisions

RevisionCommit summaryAuthorDate
r85481Revert r68358. Causes bug 27474....platonides22:38, 5 April 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r68272The rendering of autosummary in History and Diff pages did not properly escap...hartman13:40, 19 June 2010
r68342(bug 17857) Make {{anchorencode}} work in a manner more similar to the way that...conrad23:43, 20 June 2010

Comments

#Comment by Simetrical (talk | contribs)   00:42, 22 February 2011

Causes bug 27474 if $wgHtml5 is off. Test case: make sure $wgHtml5 is off but you're otherwise on default settings, create a page with 

<span id=_foo>Foo</span>

[[#_foo]]

and click the link. In r68358 it jumps to the span, after this revision it doesn't. I'm not even quite sure what this commit was meant to accomplish, so I provisionally suggest just reverting it. (If $wgHtml5 is true, then $wgExperimentalHtmlIds takes effect and Sanitizer::escapeId() strips the whitespace for you, so the bug doesn't occur.)

#Comment by Simetrical (talk | contribs)   00:45, 22 February 2011

Confirm also that reverting the commit in trunk fixes the problem. Anyone have any reasons not to do that?

#Comment by Conrad.Irwin (talk | contribs)   00:55, 22 February 2011

This was changed to make Anchors generated by the table of contents match up with section headings.

_hi

#_hi

Reverting it will cause those links to break.

#Comment by Simetrical (talk | contribs)   01:10, 22 February 2011

Yeah, confirmed. It's the headings that are broken, not the links, so you changed the wrong thing; headings shouldn't have whitespace trimmed with $wgHtml5/$wgExperimentalHtmlIds false, or at least that was the idea.

But this is all only if $wgHtml5 or $wgExperimentalHtmlIds is false. So I propose taking the easy way out:

  1. Enable $wgHtml5 on Wikimedia (Roan said he was thinking of doing this tomorrow)
  2. Wait a while to make sure that it's safe and there are no major bugs
  3. Remove the $wgHtml5 option and force it to true for the 1.17 release

That way we don't have to worry about this, or similar weird bugs that might crop up for $wgHtml5 = false. We can also then revert this commit, which I think should have no effect at that point.

#Comment by Conrad.Irwin (talk | contribs)   00:56, 22 February 2011

(bleargh, I don't mean table of contents; that was unrelated). Just anchors on normal links

Status & tagging log