Enable quick and complete cancellation of running searches #637 (#642)

www/js/app.js

@@ -52,6 +52,13 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
      */
     var cssCache = new Map();
 
+    /**
+     * A global object for storing app state
+     * 
+     * @type Object
+     */
+    var appstate = {};
+
     /**
      * @type ZIMArchive
      */
@@ -75,9 +82,8 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
     document.getElementById('appThemeSelect').value = params.appTheme;
     uiUtil.applyAppTheme(params.appTheme);
 
-    // Define global state (declared in init.js)
     // An object to hold the current search and its state (allows cancellation of search across modules)
-    globalstate['search'] = {
+    appstate['search'] = {
         'prefix': '', // A field to hold the original search string
         'status': '',  // The status of the search: ''|'init'|'interim'|'cancelled'|'complete'
         'type': ''    // The type of the search: 'basic'|'full' (set automatically in search algorithm)
@@ -119,7 +125,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
     $('#searchArticles').on('click', function() {
         var prefix = document.getElementById('prefix').value;
         // Do not initiate the same search if it is already in progress
-        if (globalstate.search.prefix === prefix && !/^(cancelled|complete)$/.test(globalstate.search.status)) return;
+        if (appstate.search.prefix === prefix && !/^(cancelled|complete)$/.test(appstate.search.status)) return;
         $("#welcomeText").hide();
         $('.alert').hide();
         $("#searchingArticles").show();
@@ -209,7 +215,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
     // Hide the search results if user moves out of prefix field
     $('#prefix').on('blur', function() {
         if (!searchArticlesFocused) {
-            globalstate.search.status = 'cancelled';
+            appstate.search.status = 'cancelled';
             $("#searchingArticles").hide();
             $('#articleListWithHeader').hide();
         }
@@ -711,7 +717,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
             }
             else if (titleSearch && titleSearch !== '') {
                 $('#prefix').val(titleSearch);
-                if (titleSearch !== globalstate.search.prefix) {
+                if (titleSearch !== appstate.search.prefix) {
                     searchDirEntriesFromPrefix(titleSearch);
                 } else {
                     $('#prefix').focus();
@@ -961,7 +967,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
         }
         window.timeoutKeyUpPrefix = window.setTimeout(function () {
             var prefix = $("#prefix").val();
-            if (prefix && prefix.length > 0 && prefix !== globalstate.search.prefix) {
+            if (prefix && prefix.length > 0 && prefix !== appstate.search.prefix) {
                 $('#searchArticles').click();
             }
         }, 500);
@@ -974,10 +980,15 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
      */
     function searchDirEntriesFromPrefix(prefix) {
         if (selectedArchive !== null && selectedArchive.isReady()) {
-            // Store the new search term in the globalstate.search object and initialize
+            // Cancel the old search (zimArchive search object will receive this change)
-            globalstate.search = {'prefix': prefix, 'status': 'init', 'type': ''};
+            appstate.search.status = 'cancelled';
+            // Initiate a new search object and point appstate.search to it (the zimArchive search object will continue to point to the old object)
+            // DEV: Technical explanation: the appstate.search is a pointer to an underlying object assigned in memory, and we are here defining a new object
+            // in memory {'prefix': prefix, 'status': 'init', .....}, and pointing appstate.search to it; the old search object that was passed to selectedArchive
+            // (zimArchive.js) continues to exist in the scope of the functions initiated by the previous search until all Promises have returned
+            appstate.search = {'prefix': prefix, 'status': 'init', 'type': ''};
             $('#activeContent').hide();
-            selectedArchive.findDirEntriesWithPrefix(globalstate.search, params.maxSearchResultsSize, populateListOfArticles);
+            selectedArchive.findDirEntriesWithPrefix(appstate.search, params.maxSearchResultsSize, populateListOfArticles);
         } else {
             $('#searchingArticles').hide();
             // We have to remove the focus from the search field,
@@ -991,23 +1002,23 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
     /**
      * Display the list of articles with the given array of DirEntry
      * @param {Array} dirEntryArray The array of dirEntries returned from the binary search
-     * @param {Object} reportingSearchPrefix The prefix of the reporting search
+     * @param {Object} reportingSearch The reporting search object
      */
-    function populateListOfArticles(dirEntryArray, reportingSearchPrefix) {
+    function populateListOfArticles(dirEntryArray, reportingSearch) {
-        // Do not allow cancelled or changed searches to report
+        // Do not allow cancelled searches to report
-        if (globalstate.search.status === 'cancelled' || globalstate.search.prefix !== reportingSearchPrefix) return;
+        if (reportingSearch.status === 'cancelled') return;
-        var stillSearching = globalstate.search.status === 'interim';
+        var stillSearching = reportingSearch.status === 'interim';
         var articleListHeaderMessageDiv = $('#articleListHeaderMessage');
         var nbDirEntry = dirEntryArray ? dirEntryArray.length : 0;
 
         var message;
         if (stillSearching) {
-            message = 'Searching [' + globalstate.search.type + ']... found: ' + nbDirEntry;
+            message = 'Searching [' + reportingSearch.type + ']... found: ' + nbDirEntry;
         } else if (nbDirEntry >= params.maxSearchResultsSize) {
             message = 'First ' + params.maxSearchResultsSize + ' articles found (refine your search).';
         } else {
             message = 'Finished. ' + (nbDirEntry ? nbDirEntry : 'No') + ' articles found' + (
-                globalstate.search.type === 'basic' ? ': try fewer words for full search.' : '.'
+                reportingSearch.type === 'basic' ? ': try fewer words for full search.' : '.'
             );
         }
 
@@ -1027,7 +1038,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
         // and prevents this event from firing; note that touch also triggers mousedown
         $('#articleList a').on('mousedown', function (e) {
             // Cancel search immediately
-            globalstate.search.status = 'cancelled';
+            appstate.search.status = 'cancelled';
             handleTitleClick(e);
             return false;
         });
@@ -1090,7 +1101,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
      */
     function readArticle(dirEntry) {
         // Reset search prefix to allow users to search the same string again if they want to
-        globalstate.search.prefix = '';
+        appstate.search.prefix = '';
         // Only update for expectedArticleURLToBeDisplayed.
         expectedArticleURLToBeDisplayed = dirEntry.namespace + "/" + dirEntry.url;
         // We must remove focus from UI elements in order to deselect whichever one was clicked (in both jQuery and SW modes),

www/js/init.js

@@ -30,13 +30,6 @@
  */
 var params = {};
 
-/**
- * A global object for storing app state
- * 
- * @type Object
- */
-var globalstate = {};
-
 require.config({
     baseUrl: 'js/lib',
     paths: {

www/js/lib/zimArchive.js

@@ -148,14 +148,12 @@ define(['zimfile', 'zimDirEntry', 'util', 'utf8'],
      * This should be enhanced when the ZIM format will be modified to store normalized titles
      * See https://phabricator.wikimedia.org/T108536
      * 
-     * @param {Object} search The current globalstate.search object
+     * @param {Object} search The current appstate.search object
      * @param {Integer} resultSize The number of dirEntries to find
      * @param {callbackDirEntryList} callback The function to call with the result
      * @param {Boolean} noInterim A flag to prevent callback until all results are ready (used in testing) 
      */
     ZIMArchive.prototype.findDirEntriesWithPrefix = function (search, resultSize, callback, noInterim) {
-        // Create a local invariable copy of the search prefix
-        const localPrefix = search.prefix;
         var that = this;
         // Establish array of initial values that must be searched first. All of these patterns are generated by the full
         // search type, and some by basic, but we need the most common patterns to be searched first, as it returns search
@@ -163,9 +161,9 @@ define(['zimfile', 'zimDirEntry', 'util', 'utf8'],
         // NB duplicates are removed before processing search array
         var startArray = [];
         // Ensure a search is done on the string exactly as typed
-        startArray.push(localPrefix);
+        startArray.push(search.prefix);
         // Normalize any spacing and make string all lowercase
-        var prefix = localPrefix.replace(/\s+/g, ' ').toLocaleLowerCase();
+        var prefix = search.prefix.replace(/\s+/g, ' ').toLocaleLowerCase();
         // Add lowercase string with initial uppercase (this is a very common pattern)
         startArray.push(prefix.replace(/^./, function (m) {
             return m.toLocaleUpperCase();
@@ -189,22 +187,22 @@ define(['zimfile', 'zimDirEntry', 'util', 'utf8'],
 
         function searchNextVariant() {
             // If user has initiated a new search, cancel this one
-            if (search.status === 'cancelled' || search.prefix !== localPrefix) return callback([], localPrefix);
+            if (search.status === 'cancelled') return callback([], search);
             if (prefixVariants.length === 0 || dirEntries.length >= resultSize) {
                 search.status = 'complete';
-                return callback(dirEntries, localPrefix);
+                return callback(dirEntries, search);
             }
             // Dynamically populate list of articles
             search.status = 'interim';
-            if (!noInterim) callback(dirEntries, localPrefix);
+            if (!noInterim) callback(dirEntries, search);
             var prefix = prefixVariants[0];
             prefixVariants = prefixVariants.slice(1);
-            that.findDirEntriesWithPrefixCaseSensitive(prefix, resultSize - dirEntries.length, localPrefix, search,
+            that.findDirEntriesWithPrefixCaseSensitive(prefix, resultSize - dirEntries.length, search,
                 function (newDirEntries, interim) {
-                    if (search.status === 'cancelled' || search.prefix !== localPrefix) return callback([], localPrefix);
+                    if (search.status === 'cancelled') return callback([], search);
                     if (interim) {// Only push interim results (else results will be pushed again at end of variant loop)                    
                         [].push.apply(dirEntries, newDirEntries);
-                        if (!noInterim && newDirEntries.length) callback(dirEntries, localPrefix);
+                        if (!noInterim && newDirEntries.length) callback(dirEntries, search);
                     } else searchNextVariant();
                 }
             );
@@ -217,14 +215,14 @@ define(['zimfile', 'zimDirEntry', 'util', 'utf8'],
      * 
      * @param {String} prefix The case-sensitive value against which dirEntry titles (or url) will be compared
      * @param {Integer} resultSize The maximum number of results to return
-     * @param {String} originalPrefix The original prefix typed by the user to initiate the local search 
+     * @param {Object} search The appstate.search object (for comparison, so that we can cancel long binary searches)
-     * @param {Object} search The globalstate.search object (for comparison, so that we can cancel long binary searches)
      * @param {callbackDirEntryList} callback The function to call with the array of dirEntries with titles that begin with prefix
      */
-    ZIMArchive.prototype.findDirEntriesWithPrefixCaseSensitive = function(prefix, resultSize, originalPrefix, search, callback) {
+    ZIMArchive.prototype.findDirEntriesWithPrefixCaseSensitive = function(prefix, resultSize, search, callback) {
         var that = this;
         util.binarySearch(0, this._file.articleCount, function(i) {
             return that._file.dirEntryByTitleIndex(i).then(function(dirEntry) {
+                if (search.status === 'cancelled') return 0;
                 if (dirEntry.namespace < 'A') return 1;
                 if (dirEntry.namespace > 'A') return -1;
                 // We should now be in namespace A
@@ -233,8 +231,9 @@ define(['zimfile', 'zimDirEntry', 'util', 'utf8'],
         }, true).then(function(firstIndex) {
             var dirEntries = [];
             var addDirEntries = function(index) {
-                if (search.status === 'cancelled' || search.prefix !== originalPrefix || index >= firstIndex + resultSize || index >= that._file.articleCount)
+                if (search.status === 'cancelled' || index >= firstIndex + resultSize || index >= that._file.articleCount) {
                     return dirEntries;
+                }
                 return that._file.dirEntryByTitleIndex(index).then(function(dirEntry) {
                     var title = dirEntry.getTitleOrUrl();
                     // Only return dirEntries with titles that actually begin with prefix