From 75edb859ce4e281a21422d55798eee8397438dc8 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 10 Jun 2017 13:37:54 +0200 Subject: [PATCH] Refactor the `selectScaleOption` function, in `Toolbar._updateUIState`, to prevent any possible future display glitches Since the localization service is now asynchronous, depending on the load the browser is under, there's a small risk that the lookup of the 'page_scale_percent' string could be delayed slightly. If the scale would change a couple of times in *very* quick succession, there's perhaps a *theoretical* possibility that the Zoom dropdown would display an incorrect value. Consider the following, somewhat contrived, theoretical example of two zoom commands being executed *right* after one another: ```javascript PDFViewerApplication.pdfViewer.currentScale = 1.23; PDFViewerApplication.pdfViewer.currentScaleValue = 'page-width'; ``` Only the `currentScale` call will currently trigger a l10n lookup in `selectScaleOption`. However, as far as I understand, there's no *guarantee* that the l10n string is resolved *before* `selectScaleOption` is called again as a result of the `currentScaleValue` call. This thus has the possibility of putting the Zoom dropdown into an inconsistent state, since it's currently updated synchronously for one code-path and asynchronously for another. To avoid these issues, I'm proposing that we *always* update the Zoom dropdown asynchronously, such that we can guarantee that the ordering is correct. --- web/toolbar.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/web/toolbar.js b/web/toolbar.js index 3e486edcf..d30f168c8 100644 --- a/web/toolbar.js +++ b/web/toolbar.js @@ -180,25 +180,25 @@ var Toolbar = (function ToolbarClosure() { } let selectScaleOption = (value, scale) => { - var options = items.scaleSelect.options; - var predefinedValueFound = false; - for (var i = 0, ii = options.length; i < ii; i++) { - var option = options[i]; - if (option.value !== value) { - option.selected = false; - continue; + let customScale = Math.round(scale * 10000) / 100; + this.l10n.get('page_scale_percent', { scale: customScale, }, + '{{scale}}%').then((msg) => { + let options = items.scaleSelect.options; + let predefinedValueFound = false; + for (let i = 0, ii = options.length; i < ii; i++) { + let option = options[i]; + if (option.value !== value) { + option.selected = false; + continue; + } + option.selected = true; + predefinedValueFound = true; } - option.selected = true; - predefinedValueFound = true; - } - if (!predefinedValueFound) { - var customScale = Math.round(scale * 10000) / 100; - this.l10n.get('page_scale_percent', { scale: customScale, }, - '{{scale}}%').then((msg) => { + if (!predefinedValueFound) { items.customScaleOption.textContent = msg; items.customScaleOption.selected = true; - }); - } + } + }); }; var pageNumber = this.pageNumber;