Skip to content

Commit

Permalink
Hopefully fix race condition in popup list generation
Browse files Browse the repository at this point in the history
My knowledge of JavaScript is limited, but it looks like we have a race
condition on the update of the global variable used for storing the
subset of languages to use when generating the list in the popup.

Turn this global into a local variable passed to the function that
generates the list. Make loadJSON() a bit less generic (now it does not
take a callback as argument, it simply knows it should call the function
for generating the list), and pass it the subset of languages instead so
it can forward it to list generation callback.

On a short test, this seems to address the popup being sometimes empty
on Linux (and always on Windows?) as described in issue #14.
  • Loading branch information
Qeole committed May 28, 2019
1 parent 462aab9 commit a238df6
Showing 1 changed file with 10 additions and 9 deletions.
19 changes: 10 additions & 9 deletions popup/languages.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

var gLangSubset = [];

/*
* Callback to ask background script whether popup should be opened.
*/
Expand All @@ -17,8 +15,11 @@ function handleResponse(aMsg) {
window.close();
return;
}
gLangSubset = aMsg.langSubset;
gLangSubset.push("auto");

let langSubset = [];
langSubset = aMsg.langSubset;
langSubset.push("auto");
return langSubset;
}
function handleError(error) {
console.log("[enlight] Error:", error);
Expand All @@ -27,7 +28,7 @@ function handleError(error) {
/*
* Parse JSON file to load the names of supported languages.
*/
function loadJSON(aCallback) {
function loadJSON(aLangSubset) {
var xobj = new XMLHttpRequest();
xobj.overrideMimeType("application/json");
xobj.open("GET", "../options/languages-list_all.json", true);
Expand All @@ -37,7 +38,7 @@ function loadJSON(aCallback) {
* Required use of an anonymous callback as .open will NOT return a value
* but simply returns undefined in asynchronous mode.
*/
aCallback(xobj.responseText);
generateList(xobj.responseText, aLangSubset);
}
};
xobj.send(null);
Expand All @@ -46,12 +47,12 @@ function loadJSON(aCallback) {
/*
* Print list of languages.
*/
function generateList(aResponse) {
function generateList(aResponse, aLangSubset) {
var languageList = JSON.parse(aResponse);
var keys = Object.keys(languageList);

for (var l of keys) {
if (!gLangSubset.includes(l))
if (!aLangSubset.includes(l))
continue;

var element = document.createElement("div");
Expand All @@ -73,7 +74,7 @@ function generateList(aResponse) {
*/
browser.runtime.sendMessage({shouldOpenPopup: "query"})
.then(handleResponse, handleError)
.then(loadJSON(generateList))
.then(loadJSON, handleError)

/*
* On user click, send selected language id to background script, then close
Expand Down

0 comments on commit a238df6

Please sign in to comment.