r97611 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r97610‎ | r97611 | r97612 >
Date:09:25, 20 September 2011
Author:questpc
Status:deferred (Comments)
Tags:
Comment:
Implemented optional propwidth attribute. layout, propwidth, textwidth attributes now can be specified for qpoll tag, applied to the questions which do not define these attributes. Truncate too long proposaltext line to UTF-8 characters bounds.
Modified paths:
  • /trunk/extensions/QPoll/clientside/qp_user.css (modified) (history)
  • /trunk/extensions/QPoll/ctrl/qp_abstractpoll.php (modified) (history)
  • /trunk/extensions/QPoll/ctrl/qp_abstractquestion.php (modified) (history)
  • /trunk/extensions/QPoll/ctrl/qp_tabularquestion.php (modified) (history)
  • /trunk/extensions/QPoll/ctrl/qp_textquestion.php (modified) (history)
  • /trunk/extensions/QPoll/qp_pollstore.php (modified) (history)
  • /trunk/extensions/QPoll/qp_user.php (modified) (history)
  • /trunk/extensions/QPoll/view/qp_stubquestionview.php (modified) (history)
  • /trunk/extensions/QPoll/view/qp_tabularquestionview.php (modified) (history)
  • /trunk/extensions/QPoll/view/qp_textquestionview.php (modified) (history)

Diff [purge]

Index: trunk/extensions/QPoll/clientside/qp_user.css
——@@ -43,7 +43,7 @@
4444 .qpoll .line_numbers { font-family: monospace, "Courier New"; white-space:pre; color:green; background-color: lightgray; float:left; border-left: 1px solid darkgray; border-top: 1px solid darkgray; border-bottom: 1px solid darkgray; padding: 0.5em; }
4545 .qpoll .script_view { font-family: monospace, "Courier New"; white-space:pre; overflow:auto; color:black; background-color: lightgray; border-right: 1px solid darkgray; border-top: 1px solid darkgray; border-bottom: 1px solid darkgray; padding: 0.5em; }
4646
47 -/* LTR part (RTL is included from separate file */
 47+/* LTR part (RTL is included from separate file) */
4848 .qpoll .attempts_counter{ border: 1px solid gray; padding: 0.1em 0.5em 0.1em 0.5em; color: black; background-color: lightblue; margin-left: 1em; }
4949 .qpoll .question { margin-left:2em; }
5050 .qpoll .margin { padding-left:20px; }
Index: trunk/extensions/QPoll/ctrl/qp_abstractpoll.php
——@@ -67,13 +67,26 @@
6868
6969 # current state of poll parsing (no error)
7070 var $mState = '';
71 - # // true, when the poll is posted (answered)
 71+ # true, when the poll is posted (answered)
7272 var $mBeingCorrected = false;
7373
74 - // qp_pollStore instance that will be used to transfer poll data from/to DB
 74+ # qp_pollStore instance that will be used to transfer poll data from/to DB
7575 var $pollStore = null;
7676
7777 /**
 78+ * default values of 'propwidth', 'textwidth' and 'layout' attributes
 79+ * will be applied to child questions that do not have these attributes defined
 80+ *
 81+ * 'showresults' currently is handled separately, because it has "multivalue"
 82+ * and can be partially merged from poll to question
 83+ */
 84+ var $defaultQuestionAttributes = array(
 85+ 'propwidth' => null,
 86+ 'layout' => null,
 87+ 'textwidth' => null
 88+ );
 89+
 90+ /**
7891 * Constructor
7992 *
8093 * @public
——@@ -102,7 +115,12 @@
103116 }
104117 $this->view->showResults = self::parseShowResults( $argv['showresults'] );
105118 }
106 -
 119+ # get default question attributes, if any
 120+ foreach ( $this->defaultQuestionAttributes as $attr => &$val ) {
 121+ if ( array_key_exists( $attr, $argv ) ) {
 122+ $val = $argv[$attr];
 123+ }
 124+ }
107125 # every poll on the page should have unique poll id, to minimize the risk of collisions
108126 # it is required to be set manually via id="value" parameter
109127 # ( used only in "declaration" mode )
——@@ -203,18 +221,23 @@
204222
205223 /**
206224 * Parses attribute line of the question
207 - * @param $attr_str attribute string from poll's header
 225+ * @param $attr_str attribute string from questions header
208226 * @modifies $paramkeys array key is attribute regexp, value is the value of attribute
209227 * @return string the value of question's type attribute
210228 */
211229 function getQuestionAttributes( $attr_str, &$paramkeys ) {
212 - $paramkeys = array( 't[yi]p[eo]' => null, 'layout' => null, 'textwidth' => null, 'showresults' => null );
 230+ $paramkeys = array( 't[yi]p[eo]' => null, 'layout' => null, 'textwidth' => null, 'propwidth' => null, 'showresults' => null );
213231 foreach ( $paramkeys as $key => &$val ) {
214232 preg_match( '`' . $key . '?="(.*?)"`u', $attr_str, $val );
 233+ $val = ( count( $val ) > 1 ) ? $val[1] : null;
215234 }
216 - $type = $paramkeys[ 't[yi]p[eo]' ];
217 - $type = isset( $type[1] ) ? trim( $type[1] ) : '';
218 - return $type;
 235+ # apply default question attributes, if any
 236+ foreach ( $this->defaultQuestionAttributes as $attr => $val ) {
 237+ if ( is_null( $paramkeys[$attr] ) ) {
 238+ $paramkeys[$attr] = $val;
 239+ }
 240+ }
 241+ return isset( $paramkeys[ 't[yi]p[eo]' ] ) ? trim( $paramkeys[ 't[yi]p[eo]' ] ) : '';
219242 }
220243
221244 // parses source showresults xml parameter value and returns the corresponding showResults array
Index: trunk/extensions/QPoll/ctrl/qp_abstractquestion.php
——@@ -89,6 +89,7 @@
9090 function applyAttributes( $paramkeys ) {
9191 $this->view->setLayout( $paramkeys[ 'layout' ], $paramkeys[ 'textwidth' ] );
9292 $this->view->setShowResults( $paramkeys[ 'showresults' ] );
 93+ $this->view->setPropWidth( $paramkeys[ 'propwidth' ] );
9394 }
9495
9596 function getPercents( $proposalId, $catId ) {
Index: trunk/extensions/QPoll/ctrl/qp_textquestion.php
——@@ -65,7 +65,7 @@
6666 * @param $token string current value of token between pipe separators
6767 * Also, _optionally_ overrides textwidth property
6868 */
69 - function setLastOption( $token ) {
 69+ function addToLastOption( $token ) {
7070 # first entry of category options might be definition of
7171 # the current category input textwidth instead
7272 $matches = array();
——@@ -76,7 +76,7 @@
7777 $this->textwidth = intval( $matches[1] );
7878 } else {
7979 # add new input option
80 - $this->iopt_last .= trim( $token );
 80+ $this->iopt_last .= $token;
8181 }
8282 }
8383
——@@ -86,7 +86,7 @@
8787 function closeCategory() {
8888 $this->isCatDef = false;
8989 # prepare new category input choice (text questions have no category names)
90 - $unique_options = array_unique( $this->input_options, SORT_STRING );
 90+ $unique_options = array_unique( array_map( 'trim', $this->input_options ), SORT_STRING );
9191 $this->input_options = array();
9292 foreach ( $unique_options as $option ) {
9393 # make sure unique elements keys are consequitive starting from 0
——@@ -189,8 +189,12 @@
190190 switch ( $token ) {
191191 case '|' :
192192 if ( $opt->isCatDef ) {
193 - $opt->addEmptyOption();
194 - $isContinue = true;
 193+ if ( count( $brace_stack ) == 1 && $brace_stack[0] === '>>' ) {
 194+ # pipe char starts new option only at top brace level,
 195+ # with angled braces
 196+ $opt->addEmptyOption();
 197+ $isContinue = true;
 198+ }
195199 }
196200 break;
197201 case '[[' :
——@@ -230,7 +234,7 @@
231235 continue;
232236 }
233237 if ( $opt->isCatDef ) {
234 - $opt->setLastOption( $token );
 238+ $opt->addToLastOption( $token );
235239 } else {
236240 # add new proposal part
237241 $this->dbtokens[] = strval( $token );
——@@ -249,16 +253,24 @@
250254 $this->viewtokens->prependErrorToken( wfMsg( 'qp_error_too_long_proposal_text' ), 'error' );
251255 }
252256 $this->mProposalText[$proposalId] = $proposal_text;
253 - # If the proposal was submitted but has _any_ unanswered category
254 - if ( $this->poll->mBeingCorrected &&
255 - ( !array_key_exists( $proposalId, $this->mProposalCategoryId ) ||
256 - count( $this->mProposalCategoryId[$proposalId] ) !== $catId )
257 - ) {
258 - # todo: apply 'error' style to the whole row
259 - $prev_state = $this->getState();
260 - $this->viewtokens->prependErrorToken( wfMsg( 'qp_error_no_answer' ), 'NA' );
261 - if ( $prev_state == '' ) {
262 - # todo: if there was no previous errors, hightlight the whole row
 257+ if ( $this->poll->mBeingCorrected ) {
 258+ # check for unanswered categories
 259+ try {
 260+ if ( !array_key_exists( $proposalId, $this->mProposalCategoryId ) ) {
 261+ # the whole line is unanswered
 262+ throw new Exception( 'qp_error' );
 263+ }
 264+ # how many categories has to be answered,
 265+ # all defined in row or at least one?
 266+ $countRequired = ($this->mSubType === 'requireAllCategories') ? $catId : 1;
 267+ if ( count( $this->mProposalCategoryId[$proposalId] ) < $countRequired ) {
 268+ throw new Exception( 'qp_error' );
 269+ }
 270+ } catch ( Exception $e ) {
 271+ if ( $e->getMessage() == 'qp_error' ) {
 272+ $prev_state = $this->getState();
 273+ $this->viewtokens->prependErrorToken( wfMsg( 'qp_error_no_answer' ), 'NA' );
 274+ }
263275 }
264276 }
265277 $this->view->addProposal( $proposalId, $this->viewtokens->tokenslist );
Index: trunk/extensions/QPoll/ctrl/qp_tabularquestion.php
——@@ -107,6 +107,9 @@
108108 # analyze previousely built "raw" categories array
109109 # Less than two categories is a syntax error.
110110 if ( $this->mType != 'mixedChoice' && count( $categories ) < 2 ) {
 111+ if ( !isset( $categories[0] ) ) {
 112+ $categories[0] = '';
 113+ }
111114 $categories[0] .= $this->view->bodyErrorMessage( wfMsg( 'qp_error_too_few_categories' ), 'error' );
112115 }
113116 foreach ( $categories as $catkey => $category ) {
Index: trunk/extensions/QPoll/qp_user.php
——@@ -138,30 +138,36 @@
139139 static $user; // User instance we got from hook parameter
140140
141141 static $questionTypes = array(
142 - 'mixed' => array(
143 - 'ctrl' => 'qp_MixedQuestion',
144 - 'view' => 'qp_TabularQuestionView',
145 - 'mType' => 'mixedChoice'
146 - ),
147 - 'unique()' => array(
 142+ '[]' => array(
148143 'ctrl' => 'qp_TabularQuestion',
149144 'view' => 'qp_TabularQuestionView',
150 - 'mType' => 'singleChoice',
151 - 'mSubType' => 'unique'
 145+ 'mType' => 'multipleChoice'
152146 ),
153147 '()' => array(
154148 'ctrl' => 'qp_TabularQuestion',
155149 'view' => 'qp_TabularQuestionView',
156150 'mType' => 'singleChoice'
157151 ),
158 - '[]' => array(
 152+ 'unique()' => array(
159153 'ctrl' => 'qp_TabularQuestion',
160154 'view' => 'qp_TabularQuestionView',
161 - 'mType' => 'multipleChoice'
 155+ 'mType' => 'singleChoice',
 156+ 'mSubType' => 'unique'
162157 ),
 158+ 'mixed' => array(
 159+ 'ctrl' => 'qp_MixedQuestion',
 160+ 'view' => 'qp_TabularQuestionView',
 161+ 'mType' => 'mixedChoice'
 162+ ),
163163 'text' => array(
164164 'ctrl' => 'qp_TextQuestion',
165165 'view' => 'qp_TextQuestionView',
 166+ 'mType' => 'textQuestion',
 167+ 'mSubType' => 'requireAllCategories'
 168+ ),
 169+ 'text!' => array(
 170+ 'ctrl' => 'qp_TextQuestion',
 171+ 'view' => 'qp_TextQuestionView',
166172 'mType' => 'textQuestion'
167173 )
168174 );
——@@ -207,6 +213,33 @@
208214 }
209215
210216 /**
 217+ * Limit the maximum length of proposal line with respect to UTF-8 character bounds
 218+ *
 219+ * Question type=text proposal lengths are additionally checked in the question controller
 220+ * because these are stored in serialized format.
 221+ * Questions of another types have their proposal texts optionally cut down, because
 222+ * the whole text of proposal is not important.
 223+ *
 224+ * @param $ptext string proposal text
 225+ * @return string proposal text either cut down or unaltered
 226+ */
 227+ private function limitProposalLength( $ptext ) {
 228+ if ( strlen( $ptext ) <= self::$proposal_max_length ) {
 229+ return $ptext;
 230+ }
 231+ for ( $curr_len = self::$proposal_max_length;/* noop */;$curr_len-- ) {
 232+ $pcut = substr( $ptext, 0, $curr_len );
 233+ if ( mb_substr( $ptext, 0, mb_strlen( $pcut, 'UTF-8' ), 'UTF-8' ) === $pcut ) {
 234+ # valid UTF-8 cut
 235+ break;
 236+ }
 237+ # will decrease the $curr_len until valid cut is achieved;
 238+ # should not be more than 5 iterations, very often 1..3
 239+ }
 240+ return $pcut;
 241+ }
 242+
 243+ /**
211244 * Autoload classes from the map provided
212245 */
213246 static function autoLoad( $map ) {
Index: trunk/extensions/QPoll/qp_pollstore.php
——@@ -867,8 +867,8 @@
868868 private function setProposals() {
869869 $insert = Array();
870870 foreach ( $this->Questions as $qkey => &$ques ) {
871 - foreach ( $ques->ProposalText as $propkey => &$ptext ) {
872 - $insert[] = array( 'pid' => $this->pid, 'question_id' => $qkey, 'proposal_id' => $propkey, 'proposal_text' => $ptext );
 871+ foreach ( $ques->ProposalText as $propkey => $ptext ) {
 872+ $insert[] = array( 'pid' => $this->pid, 'question_id' => $qkey, 'proposal_id' => $propkey, 'proposal_text' => qp_Setup::limitProposalLength( $ptext ) );
873873 }
874874 }
875875 if ( count( $insert ) > 0 ) {
Index: trunk/extensions/QPoll/view/qp_tabularquestionview.php
——@@ -92,9 +92,9 @@
9393 }
9494
9595 function setLayout( $layout, $textwidth ) {
96 - if ( count( $layout ) > 0 ) {
97 - $this->transposed = strpos( $layout[1], 'transpose' ) !== false;
98 - $this->proposalsFirst = strpos( $layout[1], 'proposals' ) !== false;
 96+ if ( $layout !== null ) {
 97+ $this->transposed = strpos( $layout, 'transpose' ) !== false;
 98+ $this->proposalsFirst = strpos( $layout, 'proposals' ) !== false;
9999 }
100100 # setup question layout parameters
101101 if ( $this->transposed ) {
——@@ -110,8 +110,8 @@
111111 $this->proposalTextStyle = 'vertical-align:middle; ';
112112 $this->proposalTextStyle .= ( $this->proposalsFirst ) ? 'padding-right: 10px;' : 'padding-left: 10px;';
113113 }
114 - if ( count( $textwidth ) > 0 ) {
115 - $textwidth = intval( $textwidth[1] );
 114+ if ( $textwidth !== null ) {
 115+ $textwidth = intval( $textwidth );
116116 if ( $textwidth > 0 ) {
117117 $this->textInputStyle = 'width:' . $textwidth . 'em;';
118118 }
——@@ -120,9 +120,9 @@
121121
122122 function setShowResults( $showresults ) {
123123 # setup question's showresults when global showresults != 0
124 - if ( qp_Setup::$global_showresults != 0 && count( $showresults ) > 0 ) {
 124+ if ( qp_Setup::$global_showresults != 0 && $showresults !== null ) {
125125 # use the value from the question
126 - $this->showResults = qp_AbstractPoll::parseShowResults( $showresults[1] );
 126+ $this->showResults = qp_AbstractPoll::parseShowResults( $showresults );
127127 # apply undefined attributes from the poll's showresults definition
128128 foreach ( $this->pollShowResults as $attr => $val ) {
129129 if ( $attr != 'type' && !isset( $this->showResults[$attr] ) ) {
——@@ -143,6 +143,24 @@
144144 }
145145
146146 /**
 147+ * Checks, whether the supplied CSS length value is valid
 148+ * @return boolean true for valid value, false otherwise
 149+ */
 150+ function isCSSLengthValid( $width ) {
 151+ preg_match( '`^\s*(\d+)(px|em|en|%|)\s*$`', $width, $matches );
 152+ return count( $matches > 1 ) && $matches[1] > 0;
 153+ }
 154+
 155+ function setPropWidth( $attr ) {
 156+ if ( $attr !== null && $this->isCSSLengthValid( $attr ) ) {
 157+ $this->propWidth = trim( $attr );
 158+ }
 159+ if ( $this->propWidth !== '' ) {
 160+ $this->proposalTextStyle .= " width:{$this->propWidth};";
 161+ }
 162+ }
 163+
 164+ /**
147165 * Builds tagarray of categories
148166 * @param $categories "raw" categories
149167 */
Index: trunk/extensions/QPoll/view/qp_stubquestionview.php
——@@ -58,6 +58,8 @@
5959 # proposal views (indexed, sortable rows)
6060 var $pview = array();
6161
 62+ var $propWidth = '';
 63+
6264 /**
6365 * @param $parser
6466 * @param $frame
——@@ -90,6 +92,10 @@
9193 /* does nothing */
9294 }
9395
 96+ function setPropWidth( $propwidth ) {
 97+ /* does nothing */
 98+ }
 99+
94100 /**
95101 * @param $tagarray array / string row to add to the question's header
96102 */
——@@ -186,6 +192,9 @@
187193 */
188194 function renderQuestion() {
189195 $output_table = array( '__tag' => 'table', '__end' => "\n", 'class' => 'object' );
 196+ if ( $this->propWidth !== '' ) {
 197+ $output_table['style'] = 'width:100%;';
 198+ }
190199 # Determine the side border color the question.
191200 if ( $this->ctrl->getState() != '' ) {
192201 if ( isset( $output_table['class'] ) ) {
——@@ -206,7 +215,7 @@
207216 );
208217 }
209218 $tags[] = array( '__tag' => 'div', 0 => $this->rtp( $this->ctrl->mCommonQuestion ) );
210 - $tags = array( '__tag' => 'div', '__end' => "\n", 'class' => 'question', $tags );
 219+ $tags = array( '__tag' => 'div', '__end' => "\n", 'class' => 'question question_mod4_' . ( $this->ctrl->usedId % 4 ), $tags );
211220 $tags[] = &$output_table;
212221 return qp_Renderer::renderTagArray( $tags );
213222 }
Index: trunk/extensions/QPoll/view/qp_textquestionview.php
——@@ -142,8 +142,8 @@
143143
144144 function setLayout( $layout, $textwidth ) {
145145 /* todo: implement vertical layout */
146 - if ( count( $textwidth ) > 0 ) {
147 - $textwidth = intval( $textwidth[1] );
 146+ if ( $textwidth !== null ) {
 147+ $textwidth = intval( $textwidth );
148148 if ( $textwidth > 0 ) {
149149 $this->textInputStyle = 'width:' . $textwidth . 'em;';
150150 }
——@@ -168,7 +168,10 @@
169169 foreach ( $viewtokens as $elem ) {
170170 if ( is_object( $elem ) ) {
171171 if ( isset( $elem->options ) ) {
172 - $className = $elem->unanswered ? 'cat_noanswer' : 'cat_part';
 172+ $className = 'cat_part';
 173+ if ( $this->ctrl->mSubType === 'requireAllCategories' && $elem->unanswered ) {
 174+ $className = 'cat_noanswer';
 175+ }
173176 # create view for the input options part
174177 if ( count( $elem->options ) === 1 ) {
175178 # one option produces html text input

Comments

#Comment by Nikerabbit (talk | contribs)   11:44, 20 September 2011

Are you aware of $lang->truncate()?

#Comment by QuestPC (talk | contribs)   15:09, 20 September 2011

I wasn't. Thank you for pointing out. Language::removeBadCharLast() is probably more efficient than my loop with mbstring functions, however I did not profile it. Anyway, the fields are read much more often than updated. I'll switch to $wgContLang->truncate() because it seems to be available in maintenance mode as well.

Status & tagging log