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.
This commit is contained in:
		
							parent
							
								
									2971f522d4
								
							
						
					
					
						commit
						75edb859ce
					
				@ -180,25 +180,25 @@ var Toolbar = (function ToolbarClosure() {
 | 
				
			|||||||
      }
 | 
					      }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      let selectScaleOption = (value, scale) => {
 | 
					      let selectScaleOption = (value, scale) => {
 | 
				
			||||||
        var options = items.scaleSelect.options;
 | 
					        let customScale = Math.round(scale * 10000) / 100;
 | 
				
			||||||
        var predefinedValueFound = false;
 | 
					        this.l10n.get('page_scale_percent', { scale: customScale, },
 | 
				
			||||||
        for (var i = 0, ii = options.length; i < ii; i++) {
 | 
					                      '{{scale}}%').then((msg) => {
 | 
				
			||||||
          var option = options[i];
 | 
					          let options = items.scaleSelect.options;
 | 
				
			||||||
          if (option.value !== value) {
 | 
					          let predefinedValueFound = false;
 | 
				
			||||||
            option.selected = false;
 | 
					          for (let i = 0, ii = options.length; i < ii; i++) {
 | 
				
			||||||
            continue;
 | 
					            let option = options[i];
 | 
				
			||||||
 | 
					            if (option.value !== value) {
 | 
				
			||||||
 | 
					              option.selected = false;
 | 
				
			||||||
 | 
					              continue;
 | 
				
			||||||
 | 
					            }
 | 
				
			||||||
 | 
					            option.selected = true;
 | 
				
			||||||
 | 
					            predefinedValueFound = true;
 | 
				
			||||||
          }
 | 
					          }
 | 
				
			||||||
          option.selected = true;
 | 
					          if (!predefinedValueFound) {
 | 
				
			||||||
          predefinedValueFound = true;
 | 
					 | 
				
			||||||
        }
 | 
					 | 
				
			||||||
        if (!predefinedValueFound) {
 | 
					 | 
				
			||||||
          var customScale = Math.round(scale * 10000) / 100;
 | 
					 | 
				
			||||||
          this.l10n.get('page_scale_percent', { scale: customScale, },
 | 
					 | 
				
			||||||
                        '{{scale}}%').then((msg) => {
 | 
					 | 
				
			||||||
            items.customScaleOption.textContent = msg;
 | 
					            items.customScaleOption.textContent = msg;
 | 
				
			||||||
            items.customScaleOption.selected = true;
 | 
					            items.customScaleOption.selected = true;
 | 
				
			||||||
          });
 | 
					          }
 | 
				
			||||||
        }
 | 
					        });
 | 
				
			||||||
      };
 | 
					      };
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      var pageNumber = this.pageNumber;
 | 
					      var pageNumber = this.pageNumber;
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user