Revert r40027 "Recommit my changes to EditPage without the regressions:"
authorBrion Vibber <brion@users.mediawiki.org>
Wed, 27 Aug 2008 00:19:13 +0000 (00:19 +0000)
committerBrion Vibber <brion@users.mediawiki.org>
Wed, 27 Aug 2008 00:19:13 +0000 (00:19 +0000)
This caused at least a regression in tab order which causes all my edits to fail:
1) Click "edit"
2) Hit "tab" to tab into edit field
3) Type something
4) Hit "tab" to tab into the comment field
5) Hit "enter" to save
Hey wait a minute, where's my edit?
When I go back and do it over, I find that I get tabbed from the edit box to the *cancel link*, so hitting enter goes back to the view page, destroying my edit.
Not good. :(

Haven't checked for other regressions; please be *very* careful with this edit page work, as seemingly small breakages can really damage the user experience and make things really badly broken.
Maybe we need a fuller UI test suite...

includes/EditPage.php

index 3492a24..c3baebb 100644 (file)
@@ -44,7 +44,6 @@ class EditPage {
 
        var $mArticle;
        var $mTitle;
-       var $action;
        var $mMetaData = '';
        var $isConflict = false;
        var $isCssJsSubpage = false;
@@ -62,8 +61,7 @@ class EditPage {
        var $allowBlankSummary = false;
        var $autoSumm = '';
        var $hookError = '';
-       #var $mPreviewTemplates;
-       var $mParserOutput;
+       var $mPreviewTemplates;
        var $mBaseRevision = false;
 
        # Form values
@@ -94,7 +92,6 @@ class EditPage {
        function EditPage( $article ) {
                $this->mArticle =& $article;
                $this->mTitle = $article->getTitle();
-               $this->action = 'submit';
 
                # Placeholders for text injection by hooks (empty per default)
                $this->editFormPageTop =
@@ -1062,29 +1059,6 @@ class EditPage {
                return true;
        }
 
-       function setHeaders() {
-               global $wgOut, $wgTitle;
-               $wgOut->setRobotPolicy( 'noindex,nofollow' );
-               if ( $this->formtype == 'preview' ) {
-                       $wgOut->setPageTitleActionText( wfMsg( 'preview' ) );
-               }
-               if ( $this->isConflict ) {
-                       $wgOut->setPageTitle( wfMsg( 'editconflict', $wgTitle->getPrefixedText() ) );
-               } elseif( $this->section != '' ) {
-                       $msg = $this->section == 'new' ? 'editingcomment' : 'editingsection';
-                       $wgOut->setPageTitle( wfMsg( $msg, $wgTitle->getPrefixedText() ) );
-               } else {
-                       # Use the title defined by DISPLAYTITLE magic word when present
-                       if( isset($this->mParserOutput)
-                        && ( $dt = $this->mParserOutput->getDisplayTitle() ) !== false ) {
-                               $title = $dt;
-                       } else {
-                               $title = $wgTitle->getPrefixedText();
-                       }
-                       $wgOut->setPageTitle( wfMsg( 'editing', $title ) );
-               }
-       }
-
        /**
         * Send the edit form and related headers to $wgOut
         * @param $formCallback Optional callable that takes an OutputPage
@@ -1108,41 +1082,46 @@ class EditPage {
 
                wfRunHooks( 'EditPage::showEditForm:initial', array( &$this ) ) ;
 
-               #need to parse the preview early so that we know which templates are used,
-               #otherwise users with "show preview after edit box" will get a blank list
-               #we parse this near the beginning so that setHeaders can do the title
-               #setting work instead of leaving it in getPreviewText
-               $previewOutput = '';
-               if ( $this->formtype == 'preview' ) {
-                       $previewOutput = $this->getPreviewText();
-               }
-
-               $this->setHeaders();
+               $wgOut->setRobotPolicy( 'noindex,nofollow' );
 
                # Enabled article-related sidebar, toplinks, etc.
                $wgOut->setArticleRelated( true );
 
+               if ( $this->formtype == 'preview' ) {
+                       $wgOut->setPageTitleActionText( wfMsg( 'preview' ) );
+               }
+
                if ( $this->isConflict ) {
+                       $s = wfMsg( 'editconflict', $wgTitle->getPrefixedText() );
+                       $wgOut->setPageTitle( $s );
                        $wgOut->addWikiMsg( 'explainconflict' );
 
                        $this->textbox2 = $this->textbox1;
                        $this->textbox1 = $this->getContent();
                        $this->edittime = $this->mArticle->getTimestamp();
                } else {
-                       if( $this->section != '' && $this->section != 'new' ) {
-                               $matches = array();
-                               if( !$this->summary && !$this->preview && !$this->diff ) {
-                                       preg_match( "/^(=+)(.+)\\1/mi",
-                                               $this->textbox1,
-                                               $matches );
-                                       if( !empty( $matches[2] ) ) {
-                                               global $wgParser;
-                                               $this->summary = "/* " .
-                                                       $wgParser->stripSectionName(trim($matches[2])) .
-                                                       " */ ";
+                       if( $this->section != '' ) {
+                               if( $this->section == 'new' ) {
+                                       $s = wfMsg('editingcomment', $wgTitle->getPrefixedText() );
+                               } else {
+                                       $s = wfMsg('editingsection', $wgTitle->getPrefixedText() );
+                                       $matches = array();
+                                       if( !$this->summary && !$this->preview && !$this->diff ) {
+                                               preg_match( "/^(=+)(.+)\\1/mi",
+                                                       $this->textbox1,
+                                                       $matches );
+                                               if( !empty( $matches[2] ) ) {
+                                                       global $wgParser;
+                                                       $this->summary = "/* " .
+                                                               $wgParser->stripSectionName(trim($matches[2])) .
+                                                               " */ ";
+                                               }
                                        }
                                }
+                       } else {
+                               $s = wfMsg( 'editing', $wgTitle->getPrefixedText() );
                        }
+                       $wgOut->setPageTitle( $s );
 
                        if ( $this->missingComment ) {
                                $wgOut->wrapWikiMsg( '<div id="mw-missingcommenttext">$1</div>',  'missingcommenttext' );
@@ -1238,7 +1217,20 @@ class EditPage {
                        $wgOut->addHTML( "</div>\n" );
                }
 
-               $q = 'action='.$this->action;
+               #need to parse the preview early so that we know which templates are used,
+               #otherwise users with "show preview after edit box" will get a blank list
+               if ( $this->formtype == 'preview' ) {
+                       $previewOutput = $this->getPreviewText();
+               }
+
+               $rows = $wgUser->getIntOption( 'rows' );
+               $cols = $wgUser->getIntOption( 'cols' );
+
+               $ew = $wgUser->getOption( 'editwidth' );
+               if ( $ew ) $ew = " style=\"width:100%\"";
+               else $ew = '';
+
+               $q = 'action=submit';
                #if ( "no" == $redirect ) { $q .= "&redirect=no"; }
                $action = $wgTitle->escapeLocalURL( $q );
 
@@ -1290,9 +1282,16 @@ class EditPage {
                $wgOut->addHTML( $this->editFormPageTop );
 
                if ( $wgUser->getOption( 'previewontop' ) ) {
-                       $this->displayPreviewArea( $previewOutput );
-                       // Spacer for the edit toolbar
-                       $wgOut->addHTML( '<p><br /></p>' );
+
+                       if ( 'preview' == $this->formtype ) {
+                               $this->showPreview( $previewOutput );
+                       } else {
+                               $wgOut->addHTML( '<div id="wikiPreview"></div>' );
+                       }
+
+                       if ( 'diff' == $this->formtype ) {
+                               $this->showDiff();
+                       }
                }
 
 
@@ -1331,7 +1330,7 @@ class EditPage {
                if( !$this->preview && !$this->diff ) {
                        $wgOut->setOnloadHandler( 'document.editform.wpTextbox1.focus()' );
                }
-               $templates = $this->getTemplates();
+               $templates = ($this->preview || $this->section != '') ? $this->mPreviewTemplates : $this->mArticle->getUsedTemplates();
                $formattedtemplates = $sk->formatTemplates( $templates, $this->preview, $this->section != '');
 
                $hiddencats = $this->mArticle->getHiddenCategories();
@@ -1342,15 +1341,11 @@ class EditPage {
                        $metadata = $this->mMetaData ;
                        $metadata = htmlspecialchars( $wgContLang->recodeForEdit( $metadata ) ) ;
                        $top = wfMsgWikiHtml( 'metadata_help' );
-                       /* ToDo: Replace with clean code */
-                       $ew = $wgUser->getOption( 'editwidth' );
-                       if ( $ew ) $ew = " style=\"width:100%\"";
-                       else $ew = '';
-                       /* /ToDo */
                        $metadata = $top . "<textarea name='metadata' rows='3' cols='{$cols}'{$ew}>{$metadata}</textarea>" ;
                }
                else $metadata = "" ;
 
+               $hidden = '';
                $recreate = '';
                if ($this->wasDeletedSinceLastEdit()) {
                        if ( 'save' != $this->formtype ) {
@@ -1359,6 +1354,7 @@ class EditPage {
                                // Hide the toolbar and edit area, use can click preview to get it back
                                // Add an confirmation checkbox and explanation.
                                $toolbar = '';
+                               $hidden = 'type="hidden" style="display:none;"';
                                $recreate = $wgOut->parse( wfMsg( 'confirmrecreate',  $this->lastDelete->user_name , $this->lastDelete->log_comment ));
                                $recreate .=
                                        "<br /><input tabindex='1' type='checkbox' value='1' name='wpRecreate' id='wpRecreate' />".
@@ -1392,27 +1388,40 @@ END
                wfRunHooks( 'EditPage::showEditForm:fields', array( &$this, &$wgOut ) );
 
                // Put these up at the top to ensure they aren't lost on early form submission
-               $this->showFormBeforeText();
+               $wgOut->addHTML( "
+<input type='hidden' value=\"" . htmlspecialchars( $this->section ) . "\" name=\"wpSection\" />
+<input type='hidden' value=\"{$this->starttime}\" name=\"wpStarttime\" />\n
+<input type='hidden' value=\"{$this->edittime}\" name=\"wpEdittime\" />\n
+<input type='hidden' value=\"{$this->scrolltop}\" name=\"wpScrolltop\" id=\"wpScrolltop\" />\n" );
+
+               $encodedtext = htmlspecialchars( $this->safeUnicodeOutput( $this->textbox1 ) );
+               if( $encodedtext !== '' ) {
+                       // Ensure there's a newline at the end, otherwise adding lines
+                       // is awkward.
+                       // But don't add a newline if the ext is empty, or Firefox in XHTML
+                       // mode will show an extra newline. A bit annoying.
+                       $encodedtext .= "\n";
+               }
 
                $wgOut->addHTML( <<<END
-{$recreate}
+$recreate
 {$commentsubject}
 {$subjectpreview}
 {$this->editFormTextBeforeContent}
+<textarea tabindex='1' accesskey="," name="wpTextbox1" id="wpTextbox1" rows='{$rows}'
+cols='{$cols}'{$ew} $hidden>{$encodedtext}</textarea>
 END
 );
-               $this->showTextbox1();
 
                $wgOut->wrapWikiMsg( "<div id=\"editpage-copywarn\">\n$1\n</div>", $copywarnMsg );
-               $wgOut->addHTML( <<<END
-{$this->editFormTextAfterWarn}
+               $wgOut->addHTML( $this->editFormTextAfterWarn );
+               $wgOut->addHTML( "
 {$metadata}
 {$editsummary}
 {$summarypreview}
 {$checkboxhtml}
 {$safemodehtml}
-END
-);
+");
 
                $wgOut->addHTML(
 "<div class='editButtons'>
@@ -1436,18 +1445,20 @@ END
                $token = htmlspecialchars( $wgUser->editToken() );
                $wgOut->addHTML( "\n<input type='hidden' value=\"$token\" name=\"wpEditToken\" />\n" );
 
-               $this->showEditTools();
+               $wgOut->addHtml( '<div class="mw-editTools">' );
+               $wgOut->addWikiMsgArray( 'edittools', array(), array( 'content' ) );
+               $wgOut->addHtml( '</div>' );
 
-               $wgOut->addHTML( <<<END
-{$this->editFormTextAfterTools}
+               $wgOut->addHTML( $this->editFormTextAfterTools );
+
+               $wgOut->addHTML( "
 <div class='templatesUsed'>
 {$formattedtemplates}
 </div>
 <div class='hiddencats'>
 {$formattedhiddencats}
 </div>
-END
-);
+");
 
                if ( $this->isConflict && wfRunHooks( 'EditPageBeforeConflictDiff', array( &$this, &$wgOut ) ) ) {
                        $wgOut->wrapWikiMsg( '==$1==', "yourdiff" );
@@ -1457,77 +1468,26 @@ END
                        $de->showDiff( wfMsg( "yourtext" ), wfMsg( "storedversion" ) );
 
                        $wgOut->wrapWikiMsg( '==$1==', "yourtext" );
-                       $this->showTextbox2();
+                       $wgOut->addHTML( "<textarea tabindex='6' id='wpTextbox2' name=\"wpTextbox2\" rows='{$rows}' cols='{$cols}'>"
+                               . htmlspecialchars( $this->safeUnicodeOutput( $this->textbox2 ) ) . "\n</textarea>" );
                }
                $wgOut->addHTML( $this->editFormTextBottom );
                $wgOut->addHTML( "</form>\n" );
                if ( !$wgUser->getOption( 'previewontop' ) ) {
-                       $this->displayPreviewArea( $previewOutput );
-               }
 
-               wfProfileOut( $fname );
-       }
+                       if ( $this->formtype == 'preview') {
+                               $this->showPreview( $previewOutput );
+                       } else {
+                               $wgOut->addHTML( '<div id="wikiPreview"></div>' );
+                       }
 
-       protected function showFormBeforeText() {
-               global $wgOut;
-               $wgOut->addHTML( "
-<input type='hidden' value=\"" . htmlspecialchars( $this->section ) . "\" name=\"wpSection\" />
-<input type='hidden' value=\"{$this->starttime}\" name=\"wpStarttime\" />\n
-<input type='hidden' value=\"{$this->edittime}\" name=\"wpEdittime\" />\n
-<input type='hidden' value=\"{$this->scrolltop}\" name=\"wpScrolltop\" id=\"wpScrolltop\" />\n" );
-       }
-       
-       protected function showTextbox1() {
-               $attribs = array( 'tabindex' => 1 );
-               
-               if( $this->wasDeletedSinceLastEdit() )
-                       $attribs['type'] = 'hidden';
-               
-               $this->showTextbox( $this->textbox1, 'wpTextbox1', $attribs );
-       }
-       
-       protected function showTextbox2() {
-               $this->showTextbox( $this->textbox2, 'wpTextbox2', array( 'tabindex' => 6 ) );
-       }
-       
-       protected function showTextbox( $content, $name, $attribs = array() ) {
-               global $wgOut, $wgUser;
-               
-               $wikitext = $this->safeUnicodeOutput( $content );
-               if( $wikitext !== '' ) {
-                       // Ensure there's a newline at the end, otherwise adding lines
-                       // is awkward.
-                       // But don't add a newline if the ext is empty, or Firefox in XHTML
-                       // mode will show an extra newline. A bit annoying.
-                       $wikitext .= "\n";
-               }
-               
-               $attribs = array(
-                       'accesskey' => ',',
-                       'id'        => $name,
-               );
-               
-               if( $wgUser->getOption( 'editwidth' ) )
-                       $attribs['style'] = 'width: 100%';
-               
-               $wgOut->addHTML( Xml::textarea(
-                       $name,
-                       $wikitext,
-                       $wgUser->getIntOption( 'cols' ), $wgUser->getIntOption( 'rows' ),
-                       $attribs ) );
-       }
+                       if ( $this->formtype == 'diff') {
+                               $this->showDiff();
+                       }
 
-       protected function displayPreviewArea( $previewOutput ) {
-               global $wgOut;
-               if ( $this->formtype == 'preview') {
-                       $this->showPreview( $previewOutput );
-               } else {
-                       $wgOut->addHTML( '<div id="wikiPreview"></div>' );
                }
 
-               if ( $this->formtype == 'diff') {
-                       $this->showDiff();
-               }
+               wfProfileOut( $fname );
        }
 
        /**
@@ -1564,19 +1524,12 @@ END
        function doLivePreviewScript() {
                global $wgOut, $wgTitle;
                $wgOut->addScriptFile( 'preview.js' );
-               $liveAction = $wgTitle->getLocalUrl( "action={$this->action}&wpPreview=true&live=true" );
+               $liveAction = $wgTitle->getLocalUrl( 'action=submit&wpPreview=true&live=true' );
                return "return !lpDoPreview(" .
                        "editform.wpTextbox1.value," .
                        '"' . $liveAction . '"' . ")";
        }
 
-       protected function showEditTools() {
-               global $wgOut;
-               $wgOut->addHtml( '<div class="mw-editTools">' );
-               $wgOut->addWikiMsgArray( 'edittools', array(), array( 'content' ) );
-               $wgOut->addHtml( '</div>' );
-       }
-
        function getLastDelete() {
                $dbr = wfGetDB( DB_SLAVE );
                $fname = 'EditPage::getLastDelete';
@@ -1616,7 +1569,8 @@ END
        function getPreviewText() {
                global $wgOut, $wgUser, $wgTitle, $wgParser, $wgLang, $wgContLang;
 
-               wfProfileIn( __METHOD__ );
+               $fname = 'EditPage::getPreviewText';
+               wfProfileIn( $fname );
 
                if ( $this->mTriedSave && !$this->mTokenOk ) {
                        if ( $this->mTokenOkExceptSuffix ) {
@@ -1650,9 +1604,9 @@ END
                        }
                        $parserOptions->setTidy(true);
                        $parserOutput = $wgParser->parse( $previewtext , $this->mTitle, $parserOptions );
-                       //$wgOut->addHTML( $parserOutput->mText );
+                       $wgOut->addHTML( $parserOutput->mText );
                        $previewHTML = '';
-               } elseif( $rt = Title::newFromRedirect( $this->textbox1 ) ) {
+               } else if( $rt = Title::newFromRedirect( $this->textbox1 ) ) {
                        $previewHTML = $this->mArticle->viewRedirect( $rt, false );
                } else {
                        $toparse = $this->textbox1;
@@ -1689,9 +1643,20 @@ END
                                        $this->mTitle, $parserOptions );
 
                        $previewHTML = $parserOutput->getText();
-                       $this->mParserOutput = $parserOutput;
                        $wgOut->addParserOutputNoText( $parserOutput );
 
+                       # ParserOutput might have altered the page title, so reset it
+                       # Also, use the title defined by DISPLAYTITLE magic word when present
+                       if( ( $dt = $parserOutput->getDisplayTitle() ) !== false ) {
+                               $wgOut->setPageTitle( wfMsg( 'editing', $dt ) );
+                       } else {
+                               $wgOut->setPageTitle( wfMsg( 'editing', $wgTitle->getPrefixedText() ) );
+                       }
+
+                       foreach ( $parserOutput->getTemplates() as $ns => $template)
+                               foreach ( array_keys( $template ) as $dbk)
+                                       $this->mPreviewTemplates[] = Title::makeTitle($ns, $dbk);
+
                        if ( count( $parserOutput->getWarnings() ) ) {
                                $note .= "\n\n" . implode( "\n\n", $parserOutput->getWarnings() );
                        }
@@ -1700,26 +1665,18 @@ END
                $previewhead = '<h2>' . htmlspecialchars( wfMsg( 'preview' ) ) . "</h2>\n" .
                        "<div class='previewnote'>" . $wgOut->parse( $note ) . "</div>\n";
                if ( $this->isConflict ) {
-                       $previewhead .='<h2>' . htmlspecialchars( wfMsg( 'previewconflict' ) ) . "</h2>\n";
+                       $previewhead.='<h2>' . htmlspecialchars( wfMsg( 'previewconflict' ) ) . "</h2>\n";
                }
 
-               wfProfileOut( __METHOD__ );
-               return $previewhead . $previewHTML;
-       }
-       
-       function getTemplates() {
-               if( $this->preview || $this->section != '' ) {
-                       $templates = array();
-                       if( !isset($this->mParserOutput) ) return $templates;
-                       foreach( $this->mParserOutput->getTemplates() as $ns => $template) {
-                               foreach( array_keys( $template ) as $dbk ) {
-                                       $templates[] = Title::makeTitle($ns, $dbk);
-                               }
-                       }
-                       return $templates;
+               if( $wgUser->getOption( 'previewontop' ) ) {
+                       // Spacer for the edit toolbar
+                       $previewfoot = '<p><br /></p>';
                } else {
-                       return $this->mArticle->getUsedTemplates();
+                       $previewfoot = '';
                }
+               
+               wfProfileOut( $fname );
+               return $previewhead . $previewHTML . $previewfoot;
        }
 
        /**
@@ -1740,8 +1697,8 @@ END
 
                # Spit out the source or the user's modified version
                if( $source !== false ) {
-                       $rows = $wgUser->getIntOption( 'rows' );
-                       $cols = $wgUser->getIntOption( 'cols' );
+                       $rows = $wgUser->getOption( 'rows' );
+                       $cols = $wgUser->getOption( 'cols' );
                        $attribs = array( 'id' => 'wpTextbox1', 'name' => 'wpTextbox1', 'cols' => $cols, 'rows' => $rows, 'readonly' => 'readonly' );
                        $wgOut->addHtml( '<hr />' );
                        $wgOut->addWikiMsg( $first ? 'blockedoriginalsource' : 'blockededitsource', $this->mTitle->getPrefixedText() );