r113028 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r113027‎ | r113028 | r113029 >
Date:13:14, 5 March 2012
Author:jdlrobson
Status:resolved (Comments)
Tags:
Comment:
provide toggle buttons in html for Wikipedia Mobile app

Older version of the Wikipedia Mobile app ( < 1.3 ) do not use
the api and instead request pages in mobile form and strip out the
html but do not run the javascript provided in MobileFrontend as a
result no toggle buttons are rendered and thus sections cannot be toggled
on or off

this fix injects these buttons into the html reversing the work in r111733

When usage of the versions of the Wikipedia Mobile app not using the api
is low we will want to completely revert this revision

Note it was not possible to serve the buttons based on the user agent as some
versions of the Wikipedia Mobile app do not serve a special user agent and it
is important we do not break toggling functionality for users of these apps
Modified paths:
  • /trunk/extensions/MobileFrontend/MobileFormatter.php (modified) (history)
  • /trunk/extensions/MobileFrontend/MobileFrontend.body.php (modified) (history)
  • /trunk/extensions/MobileFrontend/javascripts/application.js (modified) (history)
  • /trunk/extensions/MobileFrontend/javascripts/beta_application.js (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/MobileFormatter.php
——@@ -19,6 +19,7 @@
2020 */
2121 protected $title;
2222
 23+ protected $expandableSections = false;
2324 protected $mainPage = false;
2425
2526 private $headings = 0;
——@@ -134,6 +135,14 @@
135136 return $this->format;
136137 }
137138
 139+ /**
 140+ * @todo: kill with fire when there will be minimum of pre-1.1 app users remaining
 141+ * @param bool $flag
 142+ */
 143+ public function enableExpandableSections( $flag = true ) {
 144+ $this->expandableSections = $flag;
 145+ }
 146+
138147 public function setIsMainPage( $value = true ) {
139148 $this->mainPage = $value;
140149 }
——@@ -283,7 +292,7 @@
284293
285294 switch ( $this->format ) {
286295 case 'XHTML':
287 - if ( !$this->mainPage && strlen( $html ) > 4000 ) {
 296+ if ( $this->expandableSections && !$this->mainPage && strlen( $html ) > 4000 ) {
288297 $html = $this->headingTransform( $html );
289298 }
290299 break;
——@@ -348,13 +357,27 @@
349358 '&#8593;' . $backToTop ) .
350359 Html::closeElement( 'div' );
351360 // generate the HTML we are going to inject
352 - $base .= Html::openElement( 'h2',
353 - array(
354 - 'class' => 'section_heading',
355 - 'id' => 'section_' . $this->headings
356 - )
357 - );
358 - $base .=
 361+ // TODO: remove legacy code for Wikipedia Mobile app < 1.3 which is not using the api
 362+ // when usage of said apps is low
 363+ $buttons = Html::element( 'button',
 364+ array( 'class' => 'section_heading show',
 365+ 'section_id' => $this->headings ),
 366+ $show ) .
 367+ Html::element( 'button',
 368+ array( 'class' => 'section_heading hide',
 369+ 'section_id' => $this->headings ),
 370+ $hide );
 371+ if ( $this->expandableSections ) {
 372+ $h2OnClick = 'javascript:wm_toggle_section(' . $this->headings . ');';
 373+ $base .= Html::openElement( 'h2',
 374+ array( 'class' => 'section_heading',
 375+ 'id' => 'section_' . $this->headings, 'onclick' => $h2OnClick ) );
 376+ } else {
 377+ $base .= Html::openElement( 'h2',
 378+ array( 'class' => 'section_heading',
 379+ 'id' => 'section_' . $this->headings ) );
 380+ }
 381+ $base .= $buttons .
359382 Html::rawElement( 'span',
360383 array( 'id' => $headlineId ),
361384 $matches[2] ) .
Index: trunk/extensions/MobileFrontend/MobileFrontend.body.php
——@@ -18,6 +18,7 @@
1919 public static $mainPageUrl;
2020 public static $randomPageUrl;
2121 public static $format;
 22+ public static $search;
2223 public static $callback;
2324 public static $useFormat;
2425 public static $disableImages;
——@@ -417,6 +418,7 @@
418419 self::$format = $wgRequest->getText( 'format' );
419420 self::$callback = $wgRequest->getText( 'callback' );
420421 $this->wmlContext->setRequestedSegment( $wgRequest->getInt( 'seg', 0 ) );
 422+ self::$search = $wgRequest->getText( 'search' );
421423 self::$searchField = $wgRequest->getText( 'search', '' );
422424
423425 $device = new DeviceDetection();
——@@ -1059,6 +1061,11 @@
10601062 $prepend = '<p><input emptyok="true" format="*M" type="text" name="search" value="" size="16" />' .
10611063 '<do type="accept" label="' . self::$messages['mobile-frontend-search-submit'] . '">' .
10621064 '<go href="' . $wgScript . '?title=Special%3ASearch&amp;search=$(search)"></go></do></p>';
 1065+ } elseif ( $this->contentFormat == 'XHTML'
 1066+ && self::$device['supports_javascript'] === true
 1067+ && empty( self::$search ) )
 1068+ {
 1069+ $formatter->enableExpandableSections();
10631070 }
10641071 $contentHtml = $formatter->getText( 'content', $prepend );
10651072
Index: trunk/extensions/MobileFrontend/javascripts/beta_application.js
——@@ -11,6 +11,11 @@
1212 var i, search, clearSearch, results, languageSelection, a, heading,
1313 sectionHeadings = utilities( '.section_heading' );
1414 utilities( document.body ).addClass( 'jsEnabled' );
 15+
 16+ // TODO: remove in future - currently enables toggling in Wikipedia Mobile App v < 1.3
 17+ window.wm_toggle_section = wm_toggle_section;
 18+ utilities( '.section_heading button' ).remove();
 19+
1520 function openSectionHandler() {
1621 var sectionNumber = this.id ? this.id.split( '_' )[1] : -1;
1722 if( sectionNumber > -1 ) {
——@@ -33,6 +38,7 @@
3439 }
3540 for( i = 0; i < sectionHeadings.length; i++ ) {
3641 heading = sectionHeadings[i];
 42+ heading.removeAttribute( 'onclick' ); // TODO: remove any legacy onclick handlers
3743 heading.insertBefore( createButton( true ), heading.firstChild );
3844 heading.insertBefore( createButton( false ), heading.firstChild );
3945 utilities( heading ).bind( 'click', openSectionHandler );
——@@ -132,9 +138,20 @@
133139 function bind( type, handler ) {
134140 el.addEventListener( type, handler, false );
135141 }
 142+
 143+ // TODO: support single elements
 144+ function remove(els) {
 145+ var i, el;
 146+ for( i = 0; i < els.length; i++ ) {
 147+ el = els[i];
 148+ el.parentNode.removeChild(el);
 149+ }
 150+ }
 151+
136152 return {
137153 addClass: addClass,
138154 bind: bind,
 155+ remove: remove,
139156 removeClass: removeClass
140157 };
141158 }
Index: trunk/extensions/MobileFrontend/javascripts/application.js
——@@ -7,6 +7,11 @@
88 var i, results, languageSelection, a, heading,
99 sectionHeadings = utilities( '.section_heading' );
1010 utilities( document.body ).addClass( 'jsEnabled' );
 11+
 12+ // TODO: remove in future - currently enables toggling in Wikipedia Mobile App v < 1.3
 13+ window.wm_toggle_section = wm_toggle_section;
 14+ utilities( '.section_heading button' ).remove();
 15+
1116 function openSectionHandler() {
1217 var sectionNumber = this.id ? this.id.split( '_' )[1] : -1;
1318 if( sectionNumber > -1 ) {
——@@ -29,6 +34,7 @@
3035 }
3136 for( i = 0; i < sectionHeadings.length; i++ ) {
3237 heading = sectionHeadings[i];
 38+ heading.removeAttribute( 'onclick' ); // TODO: remove any legacy onclick handlers
3339 heading.insertBefore( createButton( true ), heading.firstChild );
3440 heading.insertBefore( createButton( false ), heading.firstChild );
3541 utilities( heading ).bind( 'click', openSectionHandler );
——@@ -127,9 +133,20 @@
128134 function bind( type, handler ) {
129135 el.addEventListener( type, handler, false );
130136 }
 137+
 138+ // TODO: support single elements
 139+ function remove(els) {
 140+ var i, el;
 141+ for( i = 0; i < els.length; i++ ) {
 142+ el = els[i];
 143+ el.parentNode.removeChild(el);
 144+ }
 145+ }
 146+
131147 return {
132148 addClass: addClass,
133149 bind: bind,
 150+ remove: remove,
134151 removeClass: removeClass
135152 };
136153 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r113125fix removal of legacy buttons for browsers without jquery...jdlrobson10:46, 6 March 2012
r113126minor: correct the comment...jdlrobson10:49, 6 March 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r111733attach click handlers in javascript not html...jdlrobson09:22, 17 February 2012

Comments

#Comment by YuviPanda (talk | contribs)   18:27, 5 March 2012

Just a note - it is needed for < 1.1, not 1.3 as mentioned in the JS comment

#Comment by Jdlrobson (talk | contribs)   10:49, 6 March 2012

See follow up r113125 - this actually broke several browsers without jQuery

Status & tagging log