Skip to content

Commit

Permalink
Better handling of case transformations in search (#617)
Browse files Browse the repository at this point in the history 
Fixes #179 and #184. A complete solution to cancelling some running binary searches after user has initiated a new search will be dealt with in #637.
  • Loading branch information
@Jaifroid
Jaifroid authored Jul 11, 2020
1 parent 638728a commit 823d183
Show file tree
Hide file tree
Showing 7 changed files with 232 additions and 110 deletions.
3 changes: 3 additions & 0 deletions tests/init.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
*/
'use strict';

// Define global params needed for tests to run on existing app code
var params = {};

require.config({
baseUrl: 'www/js/lib',
paths: {
Expand Down
23 changes: 13 additions & 10 deletions tests/tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@ define(['jquery', 'zimArchive', 'zimDirEntry', 'util', 'uiUtil', 'utf8'],

var localZimArchive;


/**
* Make an HTTP request for a Blob and return a Promise
*
* @param {String} url URL to download from
* @param {String} name Name to give to the Blob instance
* @returns {Promise}
* @returns {Promise<Blob>} A Promise for the Blob
*/
function makeBlobRequest(url, name) {
return new Promise(function (resolve, reject) {
Expand Down Expand Up @@ -104,15 +103,19 @@ define(['jquery', 'zimArchive', 'zimDirEntry', 'util', 'uiUtil', 'utf8'],
var float = util.readFloatFrom4Bytes(byteArray, 0);
assert.equal(float, -118.625, "the IEEE_754 float should be converted as -118.625");
});
QUnit.test("check upper/lower case variations", function(assert) {
QUnit.test("check upper/lower case variations", function (assert) {
var testString1 = "téléphone";
var testString2 = "Paris";
var testString3 = "le Couvre-chef Est sur le porte-manteaux";
var testString4 = "épée";
assert.equal(util.ucFirstLetter(testString1), "Téléphone", "The first letter should be upper-case");
assert.equal(util.lcFirstLetter(testString2), "paris", "The first letter should be lower-case");
assert.equal(util.ucEveryFirstLetter(testString3), "Le Couvre-Chef Est Sur Le Porte-Manteaux", "The first letter of every word should be upper-case");
assert.equal(util.ucFirstLetter(testString4), "Épée", "The first letter should be upper-case (with accent)");
var testString5 = '$￥€“«xριστός» †¡Ἀνέστη!”';
var testString6 = "Καλά Νερά Μαγνησίας žižek";
assert.equal(util.allCaseFirstLetters(testString1).indexOf("Téléphone") >= 0, true, "The first letter should be uppercase");
assert.equal(util.allCaseFirstLetters(testString2).indexOf("paris") >= 0, true, "The first letter should be lowercase");
assert.equal(util.allCaseFirstLetters(testString3).indexOf("Le Couvre-Chef Est Sur Le Porte-Manteaux") >= 0, true, "The first letter of every word should be uppercase");
assert.equal(util.allCaseFirstLetters(testString4).indexOf("Épée") >= 0, true, "The first letter should be uppercase (with accent)");
assert.equal(util.allCaseFirstLetters(testString5).indexOf('$￥€“«Xριστός» †¡ἀνέστη!”') >= 0, true, "First non-punctuation/non-currency Unicode letter should be uppercase, second (with breath mark) lowercase");
assert.equal(util.allCaseFirstLetters(testString6, "full").indexOf("ΚΑΛΆ ΝΕΡΆ ΜΑΓΝΗΣΊΑΣ ŽIŽEK") >= 0, true, "All Unicode letters should be uppercase");
});
QUnit.test("check removal of parameters in URL", function(assert) {
var testUrl1 = "A/question.html";
Expand Down Expand Up @@ -174,7 +177,7 @@ define(['jquery', 'zimArchive', 'zimDirEntry', 'util', 'uiUtil', 'utf8'],
assert.equal(firstDirEntry.getTitleOrUrl() , 'A Fool for You', 'First result should be "A Fool for You"');
done();
};
localZimArchive.findDirEntriesWithPrefix('A', 5, callbackFunction);
localZimArchive.findDirEntriesWithPrefix({prefix: 'A'}, 5, callbackFunction, true);
});
QUnit.test("check findDirEntriesWithPrefix 'a'", function(assert) {
var done = assert.async();
Expand All @@ -185,7 +188,7 @@ define(['jquery', 'zimArchive', 'zimDirEntry', 'util', 'uiUtil', 'utf8'],
assert.equal(firstDirEntry.getTitleOrUrl() , 'A Fool for You', 'First result should be "A Fool for You"');
done();
};
localZimArchive.findDirEntriesWithPrefix('a', 5, callbackFunction);
localZimArchive.findDirEntriesWithPrefix({prefix: 'a'}, 5, callbackFunction, true);
});
QUnit.test("check findDirEntriesWithPrefix 'blues brothers'", function(assert) {
var done = assert.async();
Expand All @@ -196,7 +199,7 @@ define(['jquery', 'zimArchive', 'zimDirEntry', 'util', 'uiUtil', 'utf8'],
assert.equal(firstDirEntry.getTitleOrUrl() , 'Blues Brothers (film)', 'First result should be "Blues Brothers (film)"');
done();
};
localZimArchive.findDirEntriesWithPrefix('blues brothers', 5, callbackFunction);
localZimArchive.findDirEntriesWithPrefix({prefix: 'blues brothers'}, 5, callbackFunction, true);
});
QUnit.test("article '(The Night Time Is) The Right Time' correctly redirects to 'Night Time Is the Right Time'", function(assert) {
var done = assert.async();
Expand Down
16 changes: 15 additions & 1 deletion www/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ <h3>Display settings</h3>
<br />
<div class="container">
<h3>Performance settings</h3>
<div class="card card-warning" id="cacheSettingsDiv">
<div class="card card-warning" id="performanceSettingsDiv">
<div class="card-header">Speed up archive access</div>
<div class="card-body">
<div class="row">
Expand Down Expand Up @@ -305,6 +305,20 @@ <h3>Performance settings</h3>
</div>
</div>
</div>
<br />
<div class="row">
<div class="col-sm-4">
<p>Select max number of search results:</p>
</div>
<div class="col-sm-8">
<div class="range">
<label>
<input type="range" class="form-control-range" min="5" max="50" id="titleSearchRange" value="25">
<span>Value: <span id="titleSearchRangeVal"></span> (<i>default 25, higher values increase search time</i>)</span>
</label>
</div>
</div>
</div>
</div>
</div>
</div>
Expand Down
99 changes: 66 additions & 33 deletions www/js/app.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@
define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesystemAccess','q'],
function($, zimArchiveLoader, uiUtil, settingsStore, abstractFilesystemAccess, Q) {

/**
* Maximum number of articles to display in a search
* @type Integer
*/
const MAX_SEARCH_RESULT_SIZE = 50;

/**
* The delay (in milliseconds) between two "keepalive" messages sent to the ServiceWorker (so that it is not stopped
* by the browser, and keeps the MessageChannel to communicate with the application)
Expand Down Expand Up @@ -70,13 +64,25 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
params['showUIAnimations'] = settingsStore.getItem('showUIAnimations') ? settingsStore.getItem('showUIAnimations') === 'true' : true;
document.getElementById('hideActiveContentWarningCheck').checked = params.hideActiveContentWarning;
document.getElementById('showUIAnimationsCheck').checked = params.showUIAnimations;
// Maximum number of article titles to return (range is 5 - 50, default 25)
params['maxSearchResultsSize'] = settingsStore.getItem('maxSearchResultsSize') || 25;
document.getElementById('titleSearchRange').value = params.maxSearchResultsSize;
document.getElementById('titleSearchRangeVal').innerHTML = params.maxSearchResultsSize;
// A global parameter that turns caching on or off and deletes the cache (it defaults to true unless explicitly turned off in UI)
params['useCache'] = settingsStore.getItem('useCache') !== 'false';
// A parameter to set the app theme and, if necessary, the CSS theme for article content (defaults to 'light')
params['appTheme'] = settingsStore.getItem('appTheme') || 'light'; // Currently implemented: light|dark|dark_invert|dark_mwInvert
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'] = {
'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)
};

// Define globalDropZone (universal drop area) and configDropZone (highlighting area on Config page)
var globalDropZone = document.getElementById('search-article');
var configDropZone = document.getElementById('configuration');
Expand Down Expand Up @@ -111,11 +117,15 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
// Define behavior of HTML elements
var searchArticlesFocused = false;
$('#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;
$("#welcomeText").hide();
$('.alert').hide();
$("#searchingArticles").show();
pushBrowserHistoryState(null, $('#prefix').val());
searchDirEntriesFromPrefix($('#prefix').val());
pushBrowserHistoryState(null, prefix);
// Initiate the search
searchDirEntriesFromPrefix(prefix);
$('.navbar-collapse').collapse('hide');
document.getElementById('prefix').focus();
// This flag is set to true in the mousedown event below
Expand Down Expand Up @@ -198,7 +208,11 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
});
// Hide the search results if user moves out of prefix field
$('#prefix').on('blur', function() {
if (!searchArticlesFocused) $('#articleListWithHeader').hide();
if (!searchArticlesFocused) {
globalstate.search.status = 'cancelled';
$("#searchingArticles").hide();
$('#articleListWithHeader').hide();
}
});
$("#btnRandomArticle").on("click", function(e) {
$('#prefix').val("");
Expand Down Expand Up @@ -349,6 +363,13 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
refreshCacheStatus();
}
});
document.getElementById('titleSearchRange').addEventListener('change', function(e) {
settingsStore.setItem('maxSearchResultsSize', e.target.value, Infinity);
params.maxSearchResultsSize = e.target.value;
});
document.getElementById('titleSearchRange').addEventListener('input', function(e) {
document.getElementById('titleSearchRangeVal').innerHTML = e.target.value;
});

/**
* Displays or refreshes the API status shown to the user
Expand Down Expand Up @@ -441,7 +462,7 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
getCacheAttributes().then(function (cache) {
document.getElementById('cacheUsed').innerHTML = cache.description;
document.getElementById('assetsCount').innerHTML = cache.count;
var cacheSettings = document.getElementById('cacheSettingsDiv');
var cacheSettings = document.getElementById('performanceSettingsDiv');
var cacheStatusPanel = document.getElementById('cacheStatusPanel');
[cacheSettings, cacheStatusPanel].forEach(function (card) {
// IE11 cannot remove more than one class from a list at a time
Expand Down Expand Up @@ -688,9 +709,13 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
if (title && !(""===title)) {
goToArticle(title);
}
else if (titleSearch && !(""===titleSearch)) {
else if (titleSearch && titleSearch !== '') {
$('#prefix').val(titleSearch);
searchDirEntriesFromPrefix($('#prefix').val());
if (titleSearch !== globalstate.search.prefix) {
searchDirEntriesFromPrefix(titleSearch);
} else {
$('#prefix').focus();
}
}
}
};
Expand Down Expand Up @@ -926,33 +951,33 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys

/**
* Handle key input in the prefix input zone
* @param {Event} evt
* @param {Event} evt The event data to handle
*/
function onKeyUpPrefix(evt) {
// Use a timeout, so that very quick typing does not cause a lot of overhead
// It is also necessary for the words suggestions to work inside Firefox OS
if(window.timeoutKeyUpPrefix) {
if (window.timeoutKeyUpPrefix) {
window.clearTimeout(window.timeoutKeyUpPrefix);
}
window.timeoutKeyUpPrefix = window.setTimeout(function() {
window.timeoutKeyUpPrefix = window.setTimeout(function () {
var prefix = $("#prefix").val();
if (prefix && prefix.length>0) {
if (prefix && prefix.length > 0 && prefix !== globalstate.search.prefix) {
$('#searchArticles').click();
}
}
,500);
}, 500);
}


/**
* Search the index for DirEntries with title that start with the given prefix (implemented
* with a binary search inside the index file)
* @param {String} prefix
* @param {String} prefix The string that must appear at the start of any title searched for
*/
function searchDirEntriesFromPrefix(prefix) {
if (selectedArchive !== null && selectedArchive.isReady()) {
// Store the new search term in the globalstate.search object and initialize
globalstate.search = {'prefix': prefix, 'status': 'init', 'type': ''};
$('#activeContent').hide();
selectedArchive.findDirEntriesWithPrefix(prefix.trim(), MAX_SEARCH_RESULT_SIZE, populateListOfArticles);
selectedArchive.findDirEntriesWithPrefix(globalstate.search, params.maxSearchResultsSize, populateListOfArticles);
} else {
$('#searchingArticles').hide();
// We have to remove the focus from the search field,
Expand All @@ -963,30 +988,34 @@ 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
*/
function populateListOfArticles(dirEntryArray) {
function populateListOfArticles(dirEntryArray, reportingSearchPrefix) {
// Do not allow cancelled or changed searches to report
if (globalstate.search.status === 'cancelled' || globalstate.search.prefix !== reportingSearchPrefix) return;
var stillSearching = globalstate.search.status === 'interim';
var articleListHeaderMessageDiv = $('#articleListHeaderMessage');
var nbDirEntry = dirEntryArray ? dirEntryArray.length : 0;

var message;
if (nbDirEntry >= MAX_SEARCH_RESULT_SIZE) {
message = 'First ' + MAX_SEARCH_RESULT_SIZE + ' articles below (refine your search).';
if (stillSearching) {
message = 'Searching [' + globalstate.search.type + ']... found: ' + nbDirEntry;
} else if (nbDirEntry >= params.maxSearchResultsSize) {
message = 'First ' + params.maxSearchResultsSize + ' articles found (refine your search).';
} else {
message = nbDirEntry + ' articles found.';
}
if (nbDirEntry === 0) {
message = 'No articles found.';
message = 'Finished. ' + (nbDirEntry ? nbDirEntry : 'No') + ' articles found' + (
globalstate.search.type === 'basic' ? ': try fewer words for full search.' : '.'
);
}

articleListHeaderMessageDiv.html(message);

var articleListDiv = $('#articleList');
var articleListDivHtml = '';
var listLength = dirEntryArray.length < MAX_SEARCH_RESULT_SIZE ? dirEntryArray.length : MAX_SEARCH_RESULT_SIZE;
var listLength = dirEntryArray.length < params.maxSearchResultsSize ? dirEntryArray.length : params.maxSearchResultsSize;
for (var i = 0; i < listLength; i++) {
var dirEntry = dirEntryArray[i];
var dirEntryStringId = uiUtil.htmlEscapeChars(dirEntry.toStringId());
Expand All @@ -997,13 +1026,15 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
// We have to use mousedown below instead of click as otherwise the prefix blur event fires first
// 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';
handleTitleClick(e);
return false;
});
$('#searchingArticles').hide();
if (!stillSearching) $('#searchingArticles').hide();
$('#articleListWithHeader').show();
}

/**
* Handles the click on the title of an article in search results
* @param {Event} event
Expand Down Expand Up @@ -1058,7 +1089,9 @@ define(['jquery', 'zimArchiveLoader', 'uiUtil', 'settingsStore','abstractFilesys
* @param {DirEntry} dirEntry The directory entry of the article to read
*/
function readArticle(dirEntry) {
// Only update for expectedArticleURLToBeDisplayed.
// Reset search prefix to allow users to search the same string again if they want to
globalstate.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),
// but we should not do this when opening the landing page (or else one of the Unit Tests fails, at least on Chrome 58)
Expand Down
7 changes: 7 additions & 0 deletions www/js/init.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@
*/
var params = {};

/**
* A global object for storing app state
*
* @type Object
*/
var globalstate = {};

require.config({
baseUrl: 'js/lib',
paths: {
Expand Down
Loading

0 comments on commit 823d183

Please sign in to comment.