MediaWiki r98583 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:< r98582‎ | r98583 (on ViewVC)‎ | r98584 >
Date:23:34, 30 September 2011
Author:brion
Status:ok (Comments)
Tags:
Comment:
* (bug 26028) Preserve initial newlines in ProofreadPage customized edit interface.

Same basic problem as bug 12130, but not fixed by that fix because ProofreadPage creates its own <textarea>s as HTML strings in JavaScript, where fixes to Html::textarea() in PHP-land have no effect. :)
Modified paths:

Diff [purge]

Index: trunk/extensions/ProofreadPage/proofread.js
——@@ -126,15 +126,15 @@
127127 '<div id="prp_header" style="">' +
128128 '<span style="color:gray;font-size:80%;line-height:100%;">' +
129129 escapeQuotesHTML( mediaWiki.msg( 'proofreadpage_header' ) ) + '</span>' +
130 - '<textarea name="wpHeaderTextbox" rows="2" cols="80" tabindex=1>' + escapeQuotesHTML( pageHeader ) + '</textarea><br />' +
 130+ '<textarea name="wpHeaderTextbox" rows="2" cols="80" tabindex=1>\n' + escapeQuotesHTML( pageHeader ) + '</textarea><br />' +
131131 '<span style="color:gray;font-size:80%;line-height:100%;">' +
132132 escapeQuotesHTML( mediaWiki.msg( 'proofreadpage_body' ) ) + '</span></div>' +
133 - '<textarea name="wpTextbox1" id="wpTextbox1" tabindex=1 style="height:' + ( self.DisplayHeight - 6 ) + 'px;">' +
 133+ '<textarea name="wpTextbox1" id="wpTextbox1" tabindex=1 style="height:' + ( self.DisplayHeight - 6 ) + 'px;">\n' +
134134 escapeQuotesHTML( pageBody ) + '</textarea>' +
135135 '<div id="prp_footer" style="">' +
136136 '<span style="color:gray;font-size:80%;line-height:100%;">' +
137137 escapeQuotesHTML( mediaWiki.msg( 'proofreadpage_footer' ) ) + '</span><br />' +
138 - '<textarea name="wpFooterTextbox" rows="2" cols="80" tabindex=1>' +
 138+ '<textarea name="wpFooterTextbox" rows="2" cols="80" tabindex=1>\n' +
139139 escapeQuotesHTML( pageFooter ) + '</textarea></div>';
140140 }
141141

Follow-up revisions

Rev.Commit summaryAuthorDate
r98584MFT r98578 (bug 12130), r98583 (bug 26028): fixes for disappearing newlines a...brion23:37, 30 September 2011
r98753MFT r98432, r98583reedy12:57, 3 October 2011
r99164Followup to r98583; ProofreadPage was inserting a new newline at the start of...brion23:01, 6 October 2011

Past revisions this follows-up on

Rev.Commit summaryAuthorDate
r98576QUnit test case to confirm consistent browser behavior with initial newlines ...brion22:16, 30 September 2011
r98578* (bug 12130) Initial newlines are now preserved correctly during editing...brion22:50, 30 September 2011

Comments

#Comment by Brion VIBBER (Talk | contribs)   23:38, 30 September 2011

Merged to 1.18; not merging to 1.18wmf1 until someone's ready to push it.

#Comment by Enomil (Talk | contribs)   21:10, 6 October 2011

It adds useless lines beetwen the noinclude from the footer and the footer text itself, see http://de.wikisource.org/wiki/Wikisource:Skriptorium#zus.C3.A4tzliche_Leerzeilen_im_Footer with difflinks

#Comment by Brion VIBBER (Talk | contribs)   21:16, 6 October 2011

My German's a bit rusty, can you provide the steps to reproduce the bug? (exact page to hit, exact things to do while editing, exact browser version used)

#Comment by Enomil (Talk | contribs)   21:27, 6 October 2011

If you edit, preview or let you show the changes at any Page: page (on german ws Seite:) it adds every time a linebreak in the footer. The page is with the scheme headertextfooter. The linebreaks from the footer and textbox are stripped away if the extension rebuild the complete page by JS (does not know if it is on oldwikisource or now in the extension itself) but not the linebreak from the footer.

It effects all wikisource which uses Proofread Page, example pages (the also last history diffs): http://de.wikisource.org/wiki/Seite:De_DZfG_1889_01_173.jpg http://en.wikisource.org/wiki/Page:Southern_Historical_Society_Papers_volume_03.djvu/216 http://fr.wikisource.org/wiki/Page:Aventures_merveilleuses_de_Huon_de_Bordeaux.djvu/35

#Comment by Enomil (Talk | contribs)   21:29, 6 October 2011

sorry, it strips the noinclude tags in the scheme: [noinclude]header[/noinclude]text[noinclude]footer[/noinclude]

#Comment by Brion VIBBER (Talk | contribs)   23:03, 6 October 2011

r99164 should fix this up. ProofreadPage was adding a newline inside the footer on every save, which was only visible on the first edit because on subsequent edits the newline got silently eaten in the textarea and then re-inserted again on save.

Moved the inserted newline from inside the footer's <noinclude> to outside it, so we still make sure there's a newline before the header but it's in a place where extra newlines get trimmed (end of body area) so it won't grow forever.

Status & tagging log

Retrieved from "http://www.mediawiki.org/wiki/Special:Code/MediaWiki/98583"
Personal tools
Namespaces

Variants
Views
Actions
Navigation
Support
Download
Development
Communication
Toolbox