r79778 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r79777‎ | r79778 | r79779 >
Date:00:17, 7 January 2011
Author:bawolff
Status:ok (Comments)
Tags:
Comment:
Comment in code says - truncate to 250 bytes, then add ellipse. but
we don't add the ellipse, so change it to add the ellipse. The
comment and code should be the same, and since we have the room
might as well add the ellipse.

This normally wouldn't affect much, since users with JS on
will be prevented from adding that long a summary on the
client side
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/EditPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/EditPage.php
——@@ -557,7 +557,7 @@
558558 }
559559
560560 # Truncate for whole multibyte characters. +5 bytes for ellipsis
561 - $this->summary = $wgLang->truncate( $request->getText( 'wpSummary' ), 250, '' );
 561+ $this->summary = $wgLang->truncate( $request->getText( 'wpSummary' ), 250 );
562562
563563 # Remove extra headings from summaries and new sections.
564564 $this->summary = preg_replace('/^\s*=+\s*(.*?)\s*=+\s*$/', '$1', $this->summary);
Index: trunk/phase3/RELEASE-NOTES
——@@ -61,6 +61,7 @@
6262 * (bug 26574) Added 'upload' to $wgRestrictionTypes, allowing upload protected
6363 pages to be queried via the API and Special:ProtectedPages, and allowing
6464 disabling upload protection by removing it from $wgRestrictionTypes.
 65+* If an edit summary exceeds 250 bytes and is truncated, add an ellipse
6566
6667 === API changes in 1.18 ===
6768 * (bug 26339) Throw warning when truncating an overlarge API result

Follow-up revisions

RevisionCommit summaryAuthorDate
r84660(follow-up 79778) Make $wgLang->truncate function consider the length of the ...bawolff02:54, 24 March 2011

Comments

#Comment by Happy-melon (talk | contribs)   22:33, 23 March 2011

What happens if a wiki makes MediaWiki:Ellipsis into a very long string?

#Comment by Bawolff (talk | contribs)   22:35, 23 March 2011

Bad things (but that was true before this commit). Basically it will just get truncated for anything that goes beyond 5 bytes (when encoded utf-8)

Actually, its worse. Last i checked one of the default localization (pt maybe) already has this string as too long by using an &nbsp;.

#Comment by Happy-melon (talk | contribs)   22:40, 23 March 2011

It wasn't as bad in the sense that it didn't apply to this particular situation. The other uses of truncate() generally don't seem to be in 'constrained' fields; it only causes an inconvenience there for the limit to be breached, not corruption.

#Comment by Hashar (talk | contribs)   09:15, 5 June 2011

We need a better fix to handle the ellipse length. Would it be possible to throw an error message when MediaWiki:Ellipsis is edit to a size longer than 5 bytes?

#Comment by Bawolff (talk | contribs)   20:38, 6 June 2011

I changed truncate to work with any ellipse length in r84660.

resetting to new.

#Comment by Hashar (talk | contribs)   21:00, 6 June 2011

Great! I have added the follow up manually.

#Comment by Duplicatebug (talk | contribs)   16:31, 7 January 2012

After r84660 you can change the 250 to 255, because now the string with ellipse is never longer as the limit.

Status & tagging log