r97556 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r97555‎ | r97556 | r97557 >
Date:20:58, 19 September 2011
Author:preilly
Status:ok (Comments)
Tags:
Comment:
fix for Bug 30985 - Mobile header expanding and menu do not work until page loads
Modified paths:
  • /trunk/extensions/MobileFrontend/javascripts/application.js (modified) (history)
  • /trunk/extensions/MobileFrontend/views/layout/application.html.php (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/javascripts/application.js
——@@ -1,79 +1,74 @@
2 -// Ideally, this would be loaded in the head and activated by a script at the bottom,
3 -// I think, rather than attached to an onload...
4 -window.onload = function() {
5 -
6 - var search = document.getElementById( 'search' );
7 - var clearSearch = document.getElementById( 'clearsearch' );
 2+var search = document.getElementById( 'search' );
 3+var clearSearch = document.getElementById( 'clearsearch' );
84
9 - initClearSearchLink();
 5+initClearSearchLink();
106
11 - function initClearSearchLink() {
12 - clearSearch.setAttribute( 'title','Clear' );
13 - clearSearch.addEventListener( 'mousedown', clearSearchBox, true );
14 - search.addEventListener( 'keyup', _handleClearSearchLink, false );
15 - }
 7+function initClearSearchLink() {
 8+ clearSearch.setAttribute( 'title','Clear' );
 9+ clearSearch.addEventListener( 'mousedown', clearSearchBox, true );
 10+ search.addEventListener( 'keyup', _handleClearSearchLink, false );
 11+}
1612
17 - function _handleClearSearchLink() {
18 - if ( clearSearch ) {
19 - if ( search.value.length > 0 ) {
20 - clearSearch.style.display = 'block';
21 - } else {
22 - clearSearch.style.display = 'none';
23 - }
24 - }
 13+function _handleClearSearchLink() {
 14+ if ( clearSearch ) {
 15+ if ( search.value.length > 0 ) {
 16+ clearSearch.style.display = 'block';
 17+ } else {
 18+ clearSearch.style.display = 'none';
 19+ }
2520 }
 21+}
2622
27 - function clearSearchBox( event ) {
28 - search.value = '';
29 - clearSearch.style.display = 'none';
30 - if ( event ) {
31 - event.preventDefault();
32 - }
 23+function clearSearchBox( event ) {
 24+ search.value = '';
 25+ clearSearch.style.display = 'none';
 26+ if ( event ) {
 27+ event.preventDefault();
3328 }
 29+}
3430
35 - // I don't think this makes sense. Loading this here means that this button
36 - // won't function until the page is loaded. It would probably be an
37 - // improvement to just attach an onclick straight into the html.
38 - document.getElementById( 'logo' ).onclick = function() {
39 - var n = document.getElementById( 'nav' ).style;
40 - n.display = n.display == 'block' ? 'none' : 'block';
41 - };
 31+// I don't think this makes sense. Loading this here means that this button
 32+// won't function until the page is loaded. It would probably be an
 33+// improvement to just attach an onclick straight into the html.
 34+document.getElementById( 'logo' ).onclick = function() {
 35+ var n = document.getElementById( 'nav' ).style;
 36+ n.display = n.display == 'block' ? 'none' : 'block';
 37+};
4238
43 - // Also problematic, not working until the page loads...
44 - for( var h = document.getElementsByTagName( 'h2' ), i = 0; i < h.length; i++ ) {
45 - if ( h[i].className == 'section_heading' ) {
46 - h[i].onclick = function() {
47 - var section_idx = parseInt( this.id.replace( /section_(\d+)/, '$1' ) );
48 - wm_toggle_section( section_idx );
49 - }
 39+// Also problematic, not working until the page loads...
 40+for( var h = document.getElementsByTagName( 'h2' ), i = 0; i < h.length; i++ ) {
 41+ if ( h[i].className == 'section_heading' ) {
 42+ h[i].onclick = function() {
 43+ var section_idx = parseInt( this.id.replace( /section_(\d+)/, '$1' ) );
 44+ wm_toggle_section( section_idx );
5045 }
51 - };
 46+ }
 47+};
5248
53 - // And this...
54 - for ( var a = document.getElementsByTagName( 'a' ), i = 0; i < a.length; i++ ) {
55 - a[i].onclick = function() {
56 - if ( this.hash.indexOf( '#' ) == 0 ) {
57 - wm_reveal_for_hash( this.hash );
58 - }
 49+// And this...
 50+for ( var a = document.getElementsByTagName( 'a' ), i = 0; i < a.length; i++ ) {
 51+ a[i].onclick = function() {
 52+ if ( this.hash.indexOf( '#' ) == 0 ) {
 53+ wm_reveal_for_hash( this.hash );
5954 }
60 - };
61 -
62 - if ( document.location.hash.indexOf( '#' ) == 0 ) {
63 - wm_reveal_for_hash( document.location.hash );
6455 }
 56+};
6557
66 - updateOrientation();
 58+if ( document.location.hash.indexOf( '#' ) == 0 ) {
 59+ wm_reveal_for_hash( document.location.hash );
 60+}
6761
68 - // Try to scroll and hide URL bar
69 - window.scrollTo( 0, 1 );
 62+updateOrientation();
7063
71 - // This is a global. I don't know why.
72 - decode = document.getElementById( 'search' );
73 - decode.value = unescape( decode.value );
74 - decode = document.getElementsByTagName( 'title' )[0];
75 - decode.innerHTML = unescape( decode.innerHTML );
76 -};
 64+// Try to scroll and hide URL bar
 65+window.scrollTo( 0, 1 );
7766
 67+// This is a global. I don't know why.
 68+decode = document.getElementById( 'search' );
 69+decode.value = unescape( decode.value );
 70+decode = document.getElementsByTagName( 'title' )[0];
 71+decode.innerHTML = unescape( decode.innerHTML );
 72+
7873 /**
7974 * updateOrientation checks the current orientation, sets the body's class
8075 * attribute to portrait, landscapeLeft, or landscapeRight,
Index: trunk/extensions/MobileFrontend/views/layout/application.html.php
——@@ -34,7 +34,6 @@
3535 }
3636 //]]>
3737 </script>
38 - <script type="text/javascript" language="javascript" src="{$wgExtensionAssetsPath}/MobileFrontend/javascripts/application.js?version=20110912T172820Z"></script>
3938 </head>
4039 <body>
4140 {$searchWebkitHtml}
——@@ -43,6 +42,7 @@
4443 {$contentHtml}
4544 </div>
4645 {$footerHtml}
 46+ <script type="text/javascript" language="javascript" src="{$wgExtensionAssetsPath}/MobileFrontend/javascripts/application.js?version=20110919T172820Z"></script>
4747 </body>
4848 </html>
4949 EOT;

Follow-up revisions

RevisionCommit summaryAuthorDate
r97557mft r97556preilly21:00, 19 September 2011
r976801.18wmf1 Merge r97533, r97556, r97563, r97592, r97654reedy23:14, 20 September 2011
r101709partial fix for bug 30985 mobile header expanding and menu do not work until ...preilly21:38, 2 November 2011

Comments

#Comment by G.Hagedorn (talk | contribs)   13:44, 22 September 2011

Can/should this be tagged for 1.18 as well?

#Comment by Krinkle (talk | contribs)   16:58, 23 September 2011

Forwarded from r97557#code-comments

#Comment by Yair rand 07:44, 20 September 2011
event.preventDefault isn't supported by all browsers.

Status & tagging log