r104128 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r104127‎ | r104128 | r104129 >
Date:02:02, 24 November 2011
Author:preilly
Status:deferred (Comments)
Tags:mobile 
Comment:
provide login support
Modified paths:
  • /trunk/extensions/MobileFrontend/MobileFrontend.i18n.php (modified) (history)
  • /trunk/extensions/MobileFrontend/MobileFrontend.php (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/MobileFrontend.i18n.php
——@@ -69,6 +69,9 @@
7070 'mobile-frontend-feedback-page' => 'Project:Mobile Extension Feedback',
7171 'mobile-frontend-leave-feedback-thanks' => 'Thanks, for your feedback!',
7272 'mobile-frontend-language' => 'Language',
 73+ 'mobile-frontend-username' => 'Username:',
 74+ 'mobile-frontend-password' => 'Password:',
 75+ 'mobile-frontend-login' => 'Log in',
7376 );
7477
7578 /** Message documentation (Message documentation)
Index: trunk/extensions/MobileFrontend/MobileFrontend.php
——@@ -170,6 +170,9 @@
171171 'mobile-frontend-leave-feedback-thanks',
172172 'mobile-frontend-search-submit',
173173 'mobile-frontend-language',
 174+ 'mobile-frontend-username',
 175+ 'mobile-frontend-password',
 176+ 'mobile-frontend-login',
174177 );
175178
176179 public $itemsToRemove = array(
——@@ -665,7 +668,7 @@
666669 wfProfileIn( __METHOD__ );
667670 $parsedUrl = parse_url( $url );
668671 // Validates value as IP address
669 - if ( !IP::isValid( $parsedUrl['host'] ) ) {
 672+ if ( isset( $parsedUrl['host'] ) && !IP::isValid( $parsedUrl['host'] ) ) {
670673 wfProfileOut( __METHOD__ );
671674 $baseUrl = $parsedUrl['scheme'] . '://' . $parsedUrl['host'];
672675 $baseUrl = str_replace( $baseUrl, '', $url );
——@@ -1005,6 +1008,7 @@
10061009
10071010 /**
10081011 * @param $html string
 1012+ * @return string
10091013 */
10101014 public function DOMParseMainPage( $html ) {
10111015 wfProfileIn( __METHOD__ );
——@@ -1061,6 +1065,62 @@
10621066 wfProfileOut( __METHOD__ );
10631067 return $contentHtml;
10641068 }
 1069+
 1070+ /**
 1071+ * @param $token string
 1072+ * @param $action string
 1073+ * @return DomElement
 1074+ */
 1075+ public function renderLogin( $token, $action ) {
 1076+ $username = self::$messages['mobile-frontend-username'];
 1077+ $password = self::$messages['mobile-frontend-password'];
 1078+ $login = self::$messages['mobile-frontend-login'];
 1079+ $form = <<<EOT
 1080+ <form name="userlogin" method="post" action="{$action}">
 1081+ <table class="user-login">
 1082+ <tbody>
 1083+ <tr>
 1084+ <td class="mw-label"><label for="wpName1">{$username}</label></td>
 1085+ </tr>
 1086+ <tr>
 1087+ <td class="mw-input"><input class="loginText" id="wpName1" tabindex="1" size="20" value="Root" name="wpName"></td>
 1088+ </tr>
 1089+ <tr>
 1090+ <td class="mw-label"><label for="wpPassword1">{$password}</label></td>
 1091+ </tr>
 1092+ <tr>
 1093+ <td class="mw-input"><input class="loginPassword" id="wpPassword1" tabindex="2" size="20" autofocus="" type="password" name="wpPassword"></td>
 1094+ </tr>
 1095+ <tr>
 1096+ <td></td>
 1097+ </tr>
 1098+ <tr>
 1099+ <td class="mw-submit"><input id="wpLoginAttempt" tabindex="9" type="submit" value="{$login}" name="wpLoginAttempt"></td>
 1100+ </tr>
 1101+ </tbody>
 1102+ </table>
 1103+ <input type="hidden" name="wpLoginToken" value="{$token}">
 1104+ </form>
 1105+EOT;
 1106+ return $this->getDomDocumentNodeByTagName( $form, 'form' );
 1107+ }
 1108+
 1109+ /**
 1110+ * @param $html string
 1111+ * @param $tagName string
 1112+ * @return DomElement
 1113+ */
 1114+ private function getDomDocumentNodeByTagName( $html, $tagName ) {
 1115+ libxml_use_internal_errors( true );
 1116+ $dom = new DOMDocument();
 1117+ $dom->loadHTML( $html );
 1118+ libxml_use_internal_errors( false );
 1119+ $dom->preserveWhiteSpace = false;
 1120+ $dom->strictErrorChecking = false;
 1121+ $dom->encoding = 'UTF-8';
 1122+ $node = $dom->getElementsByTagName( $tagName )->item(0);
 1123+ return $node;
 1124+ }
10651125
10661126 /**
10671127 * @param $html string
——@@ -1079,7 +1139,40 @@
10801140 $this->doc->encoding = 'UTF-8';
10811141
10821142 $itemToRemoveRecords = $this->parseItemsToRemove();
 1143+
 1144+ $ptLogout = $this->doc->getElementById( 'pt-logout' );
 1145+
 1146+ if ( $ptLogout ) {
 1147+ $ptLogoutLink = $ptLogout->firstChild;
 1148+ $logoutHtml = $this->doc->saveXML( $ptLogoutLink, LIBXML_NOEMPTYTAG );
 1149+ }
10831150
 1151+ if ( self::$title == 'Special:UserLogin' ) {
 1152+ $userlogin = $this->doc->getElementById( 'userloginForm' );
 1153+
 1154+ if ( $userlogin && get_class($userlogin) === 'DOMElement' ) {
 1155+ $inputs = $userlogin->getElementsByTagName( 'input' );
 1156+ $forms = $userlogin->getElementsByTagName( 'form' );
 1157+
 1158+ if ( $forms ) {
 1159+ foreach ( $forms as $form ) {
 1160+ $action = $form->getAttribute( 'action' );
 1161+ }
 1162+ }
 1163+
 1164+ foreach ( $inputs as $input ) {
 1165+ if ( $input->getAttribute( 'name' ) === 'wpLoginToken' ) {
 1166+ $wpLoginToken = $input->getAttribute( 'value' );
 1167+ }
 1168+ }
 1169+
 1170+ $firstHeading = $this->doc->getElementById( 'firstHeading' );
 1171+ if ( $firstHeading ) {
 1172+ $firstHeading->nodeValue = '';
 1173+ }
 1174+ }
 1175+ }
 1176+
10841177 // Tags
10851178
10861179 // You can't remove DOMNodes from a DOMNodeList as you're iterating
——@@ -1161,6 +1254,15 @@
11621255
11631256 $redLink->parentNode->replaceChild( $spanNode, $redLink );
11641257 }
 1258+
 1259+ if ( self::$title == 'Special:UserLogin' ) {
 1260+ if ( isset( $wpLoginToken ) && isset( $action ) && isset( $userlogin ) ) {
 1261+ $login = $this->renderLogin( $wpLoginToken, $action );
 1262+ $loginNode = $this->doc->importNode( $login, true );
 1263+ $userlogin->appendChild( $loginNode );
 1264+ }
 1265+ }
 1266+
11651267 $content = $this->doc->getElementById( 'content' );
11661268
11671269 $contentHtml = $this->doc->saveXML( $content, LIBXML_NOEMPTYTAG );

Follow-up revisions

RevisionCommit summaryAuthorDate
r104131use HTMLFormpreilly02:35, 24 November 2011

Comments

#Comment by MZMcBride (talk | contribs)   02:08, 24 November 2011

Login form code looks scary.

#Comment by Preilly (talk | contribs)   02:36, 24 November 2011

The renderLogin method now uses HTMLForm as of r104131.

#Comment by MZMcBride (talk | contribs)   02:55, 24 November 2011

Expanding my previous comment...

  • the "EOT" syntax doesn't seem to fit in with MediaWiki's style, as far as I know; I thought things where done through $wgOut (for escaping, consistency, etc.)
  • form doesn't use HTMLForm (comment at r104131)
  • not sure a separate form/logic is needed here; I'd try to call as much core code as possible for form generation like this, otherwise you risk rewriting much of the software, don't you?
    • this is apparently to get different styling; should be limited to CSS changes
    • the last time I looked, the Userlogin.php code in core was a God-awful mess, but someone may have cleaned it up; once it's cleaned up, it shouldn't be too difficult to re-use
  • ideologically, this is a big step for the mobile site (supporting non-read actions), but I assume that was already discussed

Status & tagging log