Brion VIBBER has submitted this change and it was merged. Change subject: Rewrite search as an overlay ......................................................................
Rewrite search as an overlay This also has the side effect of fixing bug 40986 and changing our autocomplete so it only matches strings at the start of a search result Change-Id: I1f35b7360dcb69f832679ed3fe58889ff3a018d5 --- M MobileFrontend.php D javascripts/modules/mf-search.js A javascripts/modules/search-2.js M less/common/mf-common.less M stylesheets/common/mf-common.css A templates/overlays/search/results.html A templates/overlays/search/search.html D tests/javascripts/modules/test_mf-search.js A tests/javascripts/modules/test_search-2.js 9 files changed, 191 insertions(+), 353 deletions(-) Approvals: Brion VIBBER: Verified; Looks good to me, approved jenkins-bot: Checked diff --git a/MobileFrontend.php b/MobileFrontend.php index fe85dd4..f84d674 100644 --- a/MobileFrontend.php +++ b/MobileFrontend.php @@ -235,6 +235,8 @@ 'overlay', 'overlays/cleanup', 'overlays/photoCopyrightDialog', + 'overlays/search/search', + 'overlays/search/results', 'photoUploader', 'photoUploadPreview', 'ctaDrawer' @@ -404,7 +406,7 @@ 'javascripts/modules/mf-watchstar.js', 'javascripts/modules/mf-photo.js', 'javascripts/modules/mainmenutweaks.js', - 'javascripts/modules/mf-search.js', + 'javascripts/modules/search-2.js', 'javascripts/modules/mf-references.js' ), 'messages' => array( @@ -455,7 +457,7 @@ 'mobile-frontend-photo-ownership-bullet-three', 'mobile-frontend-photo-upload-cta', - // for mf-search.js + // for search.js 'mobile-frontend-search-help', 'mobile-frontend-search-noresults', 'mobile-frontend-overlay-escape', diff --git a/javascripts/modules/mf-search.js b/javascripts/modules/mf-search.js deleted file mode 100644 index ba29446..0000000 --- a/javascripts/modules/mf-search.js +++ /dev/null @@ -1,272 +0,0 @@ -/* -Triggers: -* mw-mf-search-results - Context: Runs after rendering all search results - Arguments: <DomElement(ul), article title> -*/ -( function( M, $ ) { - -// FIXME: rewrite using jQuery, overlays and possibly Views -var opensearch = ( function() { - var apiUrl = '/api.php', timer = -1, typingDelay = 500, - numResults = 15, term, mfePrefix = M.prefix, - message = M.message, - search = document.getElementById( 'searchInput' ), - oldValue, - focusBlurTimeout, - u = M.utils; - - apiUrl = M.getApiUrl(); - - function removeResults( silently ) { - if ( !silently ) { - M.history.replaceHash( '#' ); - } - u( document.body ).removeClass( 'full-screen-search' ); - } - - function onfocus() { - var rrd, header; - header = document.getElementById( mfePrefix + 'header' ); - - u( document.body ).addClass( 'full-screen-search' ); - M.history.pushState( '#mw-mf-search' ); - window.scrollTo( 0, 1 ); - - rrd = document.getElementById( 'remove-results' ); - if ( !rrd ) { - rrd = document.createElement( 'button' ); - rrd.setAttribute( 'id', 'remove-results' ); - $( rrd ).addClass( 'escapeOverlay' ).on( 'click', function() { - removeResults(); - } ).text( message( 'mobile-frontend-overlay-escape' ) ); - header.insertBefore( rrd, header.firstChild ); - } - } - - // Certain symbian devices fire blur/focus events as you mouseover an element - // this can lead to lag where focus and blur handlers are continously called - // this function allows us to delay them - function waitForFocusBlur( ev, handler ) { - var ua = navigator.userAgent; - if( ua.match(/Android 2\./) || - ua.match(/Opera Mini/) ) { // timeouts do not fire on focused input in android 2 OR opera mini - handler( ev ); - } else { - window.clearTimeout( focusBlurTimeout ); - focusBlurTimeout = window.setTimeout(function() { - handler( ev ); - }, 500); - } - } - - function createObjectArray( json ) { - var sections = [], i, item, section, - items = json[ 1 ] || []; - for ( i = 0; i < items.length; i++ ) { - item = items[i]; - section = { - label: item, - value: M.history.getArticleUrl( item ) - }; - sections.push( section ); - } - return sections; - } - - function htmlEntities( str ) { - var text = document.createTextNode( str ), - el = document.createElement( 'div' ); - el.appendChild( text ); - return el.innerHTML; - } - - function escapeJsString( str ) { - return str.replace( /[\-\[\]{}()*+?.,\\\^$|#\s]/g, '\\$&' ); - } - - function printMessage( msg ) { - var results = document.getElementById( 'results' ); - results.innerHTML = '<ul class="suggestions-results" title="No Results"><li class="suggestions-result">' + - msg + '</li></div>'; - } - - function clickSearchResult( ev ) { - M.history.navigateToPage( this.getAttribute( 'title' ) ); - ev.preventDefault(); - removeResults(); - } - - function writeResults( sections ) { - var results = document.getElementById( 'results' ), suggestions, i, - term, search, - section, escapedTerm, suggestionsResult, link, label; - - search = document.getElementById( 'searchInput' ); - term = htmlEntities( search.value ); - - if ( !sections || sections.length < 1 ) { - printMessage( message( 'mobile-frontend-search-noresults' ) ); - } else { - if( results.firstChild ) { - results.removeChild( results.firstChild ); - } - suggestions = document.createElement( 'ul' ); - suggestions.className = 'suggestions-results'; - results.appendChild( suggestions ); - - for ( i = 0; i < sections.length; i++ ) { - section = sections[i]; - suggestionsResult = document.createElement( 'li' ); - suggestionsResult.setAttribute( 'title', section.label ); - suggestionsResult.className = 'suggestions-result'; - - link = document.createElement( 'a' ); - link.setAttribute( 'href', section.value ); - link.setAttribute( 'title', section.label ); - link.className = 'search-result-item'; - label = document.createTextNode( section.label ); - link.appendChild( label ); - u( link ).bind( 'click', clickSearchResult ); - suggestionsResult.appendChild( link ); - suggestions.appendChild( suggestionsResult ); - // TODO: simplify the highlighting code to not use htmlEntities - // highlight matched term - escapedTerm = escapeJsString( term ); - link.innerHTML = link.innerHTML.replace( new RegExp( '(' + escapedTerm + ')' , 'ig'), - '<strong>$1</strong>' ); - } - - if ( $ ) { - M.emit( 'search-results', $( suggestions ) ); - } - } - } - - function searchApi( term ) { - u( search ).addClass( 'searching' ); - var url = apiUrl + '?action=opensearch&limit=' + numResults + '&namespace=0&format=json&search=' + term; - u.ajax( { url: url, - dataType: 'json', - success: function( data ) { - if( u( document.body ).hasClass( 'full-screen-search' ) ) { - writeResults( createObjectArray( data ) ); - u( search ).removeClass( 'searching' ); - } - } - } ); - } - - function performSearch( ev ) { - if( ev ) { - ev.preventDefault(); - } - clearTimeout( timer ); - term = search.value; - if ( term.length > 1 ) { - term = encodeURIComponent( term ); - timer = setTimeout( function () { searchApi( term ); }, typingDelay ); - } - } - - function blurSearch(ev) { - performSearch( ev ); // for opera mini etc - } - - function enhanceElements() { - var sb = document.getElementById( mfePrefix + 'searchForm' ), - interval = window.setInterval(function() { - if( !search ) { - return window.clearInterval( interval ); - } - var value = search.value; - if( value.length > 1 && value !== oldValue ) { - oldValue = value; - u( sb ).addClass( 'notEmpty' ); - performSearch(); - } else if( !value ) { - u( sb ).removeClass( 'notEmpty' ); - } - }, typingDelay); - - u( search ).bind( 'focus', - function( ev ) { - waitForFocusBlur( ev, onfocus ); - }); - u( search ).bind( 'blur', - function( ev ) { - waitForFocusBlur( ev, blurSearch ); - }); - // ensure pressing enter triggers full text search (bug 37945) - u( search ).bind( 'keyup', function( ev ) { - if ( ev.keyCode && ev.keyCode === 13 ) { - sb.submit(); - } - } ); - } - - function initClearSearch() { - var $clearSearch, - results = document.getElementById( 'results' ), - search = document.getElementById( 'searchInput' ); - - $clearSearch = $( '<a class="clearlink">' ).appendTo( '#mw-mf-sq' ); - function clearSearchBox( event ) { - // clicking clear on some browsers triggers blur event on search - // when search value empty string hides results - window.setTimeout( function() { - search.value = ''; - }, 100 ); - results.innerHTML = ''; - event.preventDefault(); - } - - function onFocusHandler() { - search.select(); - } - $clearSearch.on( 'mousedown', clearSearchBox ); - u( search ).bind( 'click', onFocusHandler ); - } - - function initSearch() { - $( '<div id="results">' ).insertAfter( '#mw-mf-header' ); - enhanceElements(); - if( document.activeElement && document.activeElement.id === 'search' ) { - onfocus(); - } - function hideKeyboard() { - document.getElementById( 'search' ).blur(); - } - document.getElementById( 'results' ).ontouchstart = hideKeyboard; - printMessage( message( 'mobile-frontend-search-help' ) ); - initClearSearch(); - u( window ).bind( 'mw-mf-history-change', function( ev, curPage ) { - if ( curPage.hash === '#mw-mf-search' ) { - onfocus(); - } else { - removeResults( true ); - } - } ); - } - - function init() { - if ( document.getElementById( 'searchInput' ) ) { - initSearch(); - } - } - - return { - init: init, - initClearSearch: initClearSearch, - writeResults: writeResults, - createObjectArray: createObjectArray, - removeResults: removeResults - }; - -}()); - -if ( typeof JSON !== 'undefined' ) { - M.define( 'opensearch', opensearch ); -} - -}( mw.mobileFrontend, jQuery )); diff --git a/javascripts/modules/search-2.js b/javascripts/modules/search-2.js new file mode 100644 index 0000000..daadb98 --- /dev/null +++ b/javascripts/modules/search-2.js @@ -0,0 +1,122 @@ +( function( M, $ ) { + +var Overlay = M.require( 'navigation' ).Overlay, SearchOverlay, + api = M.require( 'api' ), + overlayInitialize = Overlay.prototype.initialize; + +/** + * Escapes regular expression wildcards (metacharacters) by adding a \\ prefix + * @param {String} str a string + * @return {String} a regular expression that can be used to search for that str + */ +function createSearchRegEx( str ) { + str = str.replace( /[\-\[\]{}()*+?.,\\\^$|#\s]/g, '\\$&' ); + return new RegExp( '^(' + str + ')' , 'ig' ); +} + +/** + * Takes a label potentially beginning with term + * and highlights term if it is present with strong + * + * @param {String} label a piece of text + * @param {String} term a string to search for from the start + * @return {String} safe html string with matched terms encapsulated in strong tags + */ +function highlightSearchTerm( label, term ) { + label = $( '<span>' ).text( label ).html(); + term = $( '<span>' ).text( term ).html(); + + return label.replace( createSearchRegEx( term ),'<strong>$1</strong>' ); +} + +SearchOverlay = Overlay.extend( { + templateResults: M.template.get( 'overlays/search/results' ), + template: M.template.get( 'overlays/search/search' ), + defaults: { + explanation: mw.msg( 'mobile-frontend-search-help' ), + noresults: mw.msg( 'mobile-frontend-search-noresults' ), + action: $( '#mw-mf-searchForm' ).attr( 'action' ) + }, + initialize: function() { + overlayInitialize.apply( this, arguments ); + var self = this; + + this.data = this.defaults; + + this.$( 'input' ).on( 'keyup', function( ev ) { + if ( ev.keyCode && ev.keyCode === 13 ) { + $( this ).parents( 'form' ).submit(); + } else { + self.performSearch(); + } + } ); + this.results = []; + }, + /** + * A wrapper for $.ajax() to be used when calling server APIs. + * Sends a GET request. See ajax() for details. + * + * @param {Array} results list of search results with label and url properties set + */ + writeResults: function( results ) { + this.data.results = results || []; + this.$( 'ul.suggestions-results' ). + html( this.templateResults.render( this.data ) ); + + if ( results ) { + if ( results.length > 0 ) { + this.$( '.no-results' ).remove(); + } + } else { + this.$( '.no-results' ).remove(); + } + }, + performSearch: function() { + var self = this, + term = this.$( 'input' ).val(); + + function completeSearch( data ) { + data = $.map( data[ 1 ], function( item ) { + return { + label: highlightSearchTerm( item, term ), + url: M.history.getArticleUrl( item ) + }; + } ); + + self.writeResults( data ); + self.$( 'input' ).removeClass( 'searching' ); + } + + clearTimeout( this.timer ); + if ( term.length > 0 ) { + this.timer = setTimeout( function () { + self.$( 'input' ).addClass( 'searching' ); + api.get( { + search: term, + action: 'opensearch', + namespace: 0, + limit: 15 + } ).done( completeSearch ); + }, 500 ); + } + }, + showAndFocus: function() { + this.show(); + this.$( 'input' ).focus().select(); + } +} ); + +function init() { + var searchOverlay = new SearchOverlay(); + $( '#searchInput' ).on( 'focus', function() { + searchOverlay.showAndFocus(); + } ); +} +init(); + +M.define( 'search', { + SearchOverlay: SearchOverlay, + highlightSearchTerm: highlightSearchTerm +} ); + +}( mw.mobileFrontend, jQuery )); diff --git a/less/common/mf-common.less b/less/common/mf-common.less index 3fc1e50..c3efd95 100644 --- a/less/common/mf-common.less +++ b/less/common/mf-common.less @@ -309,6 +309,7 @@ padding: 0 !important; } +strong, b { font-weight: bold; } diff --git a/stylesheets/common/mf-common.css b/stylesheets/common/mf-common.css index 7460ab0..0ab178d 100644 --- a/stylesheets/common/mf-common.css +++ b/stylesheets/common/mf-common.css @@ -261,6 +261,7 @@ background: none !important; padding: 0 !important; } +strong, b { font-weight: bold; } diff --git a/templates/overlays/search/results.html b/templates/overlays/search/results.html new file mode 100644 index 0000000..71c0ca6 --- /dev/null +++ b/templates/overlays/search/results.html @@ -0,0 +1,6 @@ +<li class="no-results">{{noresults}}</li> +{{#results}} + <li> + <a href="{{url}}">{{{label}}}</a> + </li> +{{/results}} diff --git a/templates/overlays/search/search.html b/templates/overlays/search/search.html new file mode 100644 index 0000000..bbabe71 --- /dev/null +++ b/templates/overlays/search/search.html @@ -0,0 +1,10 @@ +<div class="header"> + <button class="cancel escapeOverlay">{{closeMsg}}</button> + <form class="search-box" method="get" action="{{action}}"> + <input type="search" class="search" name="search"> + </form> + <a class="clearlink"></a> +</div> +<ul class="suggestions-results"> + <li>{{explanation}}</li> +</ul> diff --git a/tests/javascripts/modules/test_mf-search.js b/tests/javascripts/modules/test_mf-search.js deleted file mode 100644 index bf61569..0000000 --- a/tests/javascripts/modules/test_mf-search.js +++ /dev/null @@ -1,79 +0,0 @@ -(function ( $, MFE, MFEOS ) { -QUnit.module( 'MobileFrontend: mf-search.js - test highlight', { - setup: function() { - $( '<form id="mw-mf-searchForm"><input id="searchInput"></form>' ).appendTo( document.body ); - $( '<div id="results">' ).appendTo( document.body ); - MFEOS.init(); - }, - teardown: function() { - $( '#mw-mf-searchForm, #results' ).remove(); - } -}); - -QUnit.test( 'no results', 1, function() { - MFEOS.writeResults([]); - strictEqual($("#results").text(), MFE.message( 'mobile-frontend-search-noresults' ) ); -}); - -QUnit.test( 'writeResults with highlighted text (case differs)', 3, function() { - var results = [ - { label: "Hello world", value: "/HelloWorld" }, - { label: "Hello kitty", value: "/HelloKitty" } - ], pageLink, pageLink2; - $( '#searchInput' ).val( 'el' ); - $( '#searchInput' ).trigger( 'keyup' ); - MFEOS.writeResults(results); - pageLink = $( '#results .suggestions-result a.search-result-item' )[ 0 ]; - pageLink2 = $( '#results .suggestions-result a.search-result-item' )[ 1 ]; - strictEqual($(pageLink).text(), "Hello world", "check the label is correct"); - strictEqual($(pageLink).html(), "H<strong>el</strong>lo world", "check the highlight is correct"); - strictEqual($(pageLink2).html(), "H<strong>el</strong>lo kitty", "check the highlight is correct"); -}); - -QUnit.test( 'writeResults with highlighted text (case differs)', 1, function() { - var results = [ - { label: "Hello world", value: "/HelloWorld" }, - { label: "Hello kitty", value: "/HelloKitty" } - ], pageLink; - $( '#searchInput' ).val( 'hel' ); - $( '#searchInput' ).trigger( 'keyup' ); - MFEOS.writeResults(results); - pageLink = $( '#results .suggestions-result a.search-result-item' )[ 0 ]; - strictEqual($(pageLink).html(), "<strong>Hel</strong>lo world", "check the highlight is correct"); -}); - -QUnit.test( 'writeResults with highlighted text (special character &)', 1, function() { - var results = [ - { label: "Belle & Sebastian", value: "/B1" }, - { label: "Belle & the Beast", value: "/B2" } - ], pageLink; - $( '#searchInput' ).val( 'Belle & S' ); - $( '#searchInput' ).trigger( 'keyup' ); - MFEOS.writeResults(results); - pageLink = $( '#results .suggestions-result a.search-result-item' )[ 0 ]; - strictEqual($(pageLink).html(), "<strong>Belle & S</strong>ebastian", "check the highlight is correct"); -}); - -QUnit.test( 'writeResults with highlighted text (special character ?)', 1, function() { - var results = [ - { label: "Title with ? in it", value: "/B1" } - ], pageLink; - $( '#searchInput' ).val( 'with ?' ); - $( '#searchInput' ).trigger( 'keyup' ); - MFEOS.writeResults(results); - pageLink = $( '#results .suggestions-result a.search-result-item' )[ 0 ]; - strictEqual($(pageLink).html(), "Title <strong>with ?</strong> in it", "check the highlight is correct"); -}); - -QUnit.test( 'writeResults with highlighted text (safe)', 1, function() { - var results = [ - { label: "<script>alert('FAIL')</script> should be safe", value: "/B1" } - ], pageLink; - $( '#searchInput' ).val( "<script>alert('FAIL'" ); - $( '#searchInput' ).trigger( 'keyup' ); - MFEOS.writeResults(results); - pageLink = $( '#results .suggestions-result a.search-result-item' )[ 0 ]; - strictEqual($(pageLink).html(), - "<strong><script>alert('FAIL'</strong>)</script> should be safe", "check the highlight is correct"); -}); -}( jQuery, mw.mobileFrontend, mw.mobileFrontend.require( 'opensearch' ) ) ); diff --git a/tests/javascripts/modules/test_search-2.js b/tests/javascripts/modules/test_search-2.js new file mode 100644 index 0000000..8b283b1 --- /dev/null +++ b/tests/javascripts/modules/test_search-2.js @@ -0,0 +1,47 @@ +( function ( $, M, s ) { +QUnit.module( 'MobileFrontend: mf-search-2.js - test highlight' ); + +QUnit.test( 'no results', 1, function() { + var overlay = new s.SearchOverlay(); + overlay.writeResults( [] ); + strictEqual( overlay.$( 'li.no-results' ).length, 1, 'Only no results list item present' ); +} ) ; + +QUnit.test( 'results', 5, function() { + var overlay = new s.SearchOverlay(); + overlay.writeResults( [ + { label: 'Hello World', url: '/hello world' }, + { label: 'Goodbye', url: '/goodbye' } + ] ); + strictEqual( overlay.$( 'li.no-results' ).length, 0, 'No no results list item present' ); + strictEqual( overlay.$( 'li' ).length, 2, '2 results items' ); + strictEqual( overlay.$( 'li a[href="/hello world"]' ).length, 1, '1 link with correct url' ); + strictEqual( overlay.$( 'li a[href="/goodbye"]' ).length, 1, '1 link with correct url' ); + strictEqual( overlay.$( 'li a[href="/goodbye"]' ).text(), 'Goodbye', 'check label of link' ); +} ) ; + +QUnit.test( 'highlightSearchTerm', 14, function() { + var data = [ + [ 'Hello World', 'Hel', '<strong>Hel</strong>lo World' ], + [ 'Hello kitty', 'el', 'Hello kitty' ], // not at start + [ 'Hello worl', 'hel', '<strong>Hel</strong>lo worl' ], + [ 'Belle & Sebastian', 'Belle & S', '<strong>Belle & S</strong>ebastian' ], + [ 'Belle & the Beast', 'Belle &', 'Belle & the Beast' ], + [ 'with ? in it', 'with ?', '<strong>with ?</strong> in it' ], // not at start + [ 'Title with ? in it', 'with ?', 'Title with ? in it' ], // not at start + [ 'AT&T', 'a', '<strong>A</strong>T&T'], + [ 'AT&T', 'at&', '<strong>AT&</strong>T'], + [ '<tag', '<tag', '<tag' ], + [ '& this is a weird title', '&', '<strong>&</strong> this is a weird title' ], + [ '& this is a weird title', '&a', '& this is a weird title' ], + [ '<t', '<t', '&lt;t' ], + [ "<script>alert('FAIL')</script> should be safe", + "<script>alert('FAIL'", "<strong><script>alert('FAIL'</strong>)</script> should be safe" ] + ]; + + $.each( data, function( i, item ) { + strictEqual( s.highlightSearchTerm( item[0], item[1] ), item[2], 'highlightSearchTerm test ' + i ); + } ); +} ); + +}( jQuery, mw.mobileFrontend, mw.mobileFrontend.require( 'search' ) ) ); -- To view, visit https://gerrit.wikimedia.org/r/58332 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1f35b7360dcb69f832679ed3fe58889ff3a018d5 Gerrit-PatchSet: 8 Gerrit-Project: mediawiki/extensions/MobileFrontend Gerrit-Branch: master Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org> Gerrit-Reviewer: Brion VIBBER <br...@wikimedia.org> Gerrit-Reviewer: JGonera <jgon...@wikimedia.org> Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org> Gerrit-Reviewer: Ori.livneh <o...@wikimedia.org> Gerrit-Reviewer: Robmoen <rm...@wikimedia.org> Gerrit-Reviewer: Yuvipanda <yuvipa...@gmail.com> Gerrit-Reviewer: awjrichards <aricha...@wikimedia.org> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits