Attempt to tweak the error messages, in BaseViewer, for invalid pageNumbers/pageLabels (bug 1505824)

Rather than closing [bug 1505824](https://bugzilla.mozilla.org/show_bug.cgi?id=1505824) as WONTFIX (which is my preferred solution), given how *minor* this "problem" is, it's still possible to adjust the error messages a bit.

The main point here, which is relevant even if the changes in `BaseViewer` are ultimately rejected during review, is that we'll no longer attempt to call `BaseViewer.currentPageLabel` with an empty string from `webViewerPageNumberChanged` in `app.js`.

The other changes are:
 - Stop printing an error in `BaseViewer._setCurrentPageNumber`, and have it return a boolean indicating if the page is within bounds.
 - Have the `BaseViewer.{currentPageNumber, currentPageLabel}` setters print their own errors for invalid pages.
 - Have the `BaseViewer.currentPageLabel` setter no longer depend, indirectly, on the `BaseViewer.currentPageNumber` setter.
 - Improve a couple of other error messages.
This commit is contained in:
Jonas Jenwald 2018-11-14 16:16:09 +01:00
parent e3e7ccc3e2
commit 9a47a86565
2 changed files with 26 additions and 12 deletions

View File

@ -1905,7 +1905,11 @@ function webViewerZoomOut() {
} }
function webViewerPageNumberChanged(evt) { function webViewerPageNumberChanged(evt) {
let pdfViewer = PDFViewerApplication.pdfViewer; let pdfViewer = PDFViewerApplication.pdfViewer;
pdfViewer.currentPageLabel = evt.value; // Note that for `<input type="number">` HTML elements, an empty string will
// be returned for non-number inputs; hence we simply do nothing in that case.
if (evt.value !== '') {
pdfViewer.currentPageLabel = evt.value;
}
// Ensure that the page number input displays the correct value, even if the // Ensure that the page number input displays the correct value, even if the
// value entered by the user was invalid (e.g. a floating point number). // value entered by the user was invalid (e.g. a floating point number).

View File

@ -212,10 +212,14 @@ class BaseViewer {
return; return;
} }
// The intent can be to just reset a scroll position and/or scale. // The intent can be to just reset a scroll position and/or scale.
this._setCurrentPageNumber(val, /* resetCurrentPageView = */ true); if (!this._setCurrentPageNumber(val, /* resetCurrentPageView = */ true)) {
console.error(
`${this._name}.currentPageNumber: "${val}" is not a valid page.`);
}
} }
/** /**
* @return {boolean} Whether the pageNumber is valid (within bounds).
* @private * @private
*/ */
_setCurrentPageNumber(val, resetCurrentPageView = false) { _setCurrentPageNumber(val, resetCurrentPageView = false) {
@ -223,13 +227,11 @@ class BaseViewer {
if (resetCurrentPageView) { if (resetCurrentPageView) {
this._resetCurrentPageView(); this._resetCurrentPageView();
} }
return; return true;
} }
if (!(0 < val && val <= this.pagesCount)) { if (!(0 < val && val <= this.pagesCount)) {
console.error( return false;
`${this._name}._setCurrentPageNumber: "${val}" is out of bounds.`);
return;
} }
this._currentPageNumber = val; this._currentPageNumber = val;
@ -242,6 +244,7 @@ class BaseViewer {
if (resetCurrentPageView) { if (resetCurrentPageView) {
this._resetCurrentPageView(); this._resetCurrentPageView();
} }
return true;
} }
/** /**
@ -256,14 +259,21 @@ class BaseViewer {
* @param {string} val - The page label. * @param {string} val - The page label.
*/ */
set currentPageLabel(val) { set currentPageLabel(val) {
let pageNumber = val | 0; // Fallback page number. if (!this.pdfDocument) {
return;
}
let page = val | 0; // Fallback page number.
if (this._pageLabels) { if (this._pageLabels) {
let i = this._pageLabels.indexOf(val); let i = this._pageLabels.indexOf(val);
if (i >= 0) { if (i >= 0) {
pageNumber = i + 1; page = i + 1;
} }
} }
this.currentPageNumber = pageNumber; // The intent can be to just reset a scroll position and/or scale.
if (!this._setCurrentPageNumber(page, /* resetCurrentPageView = */ true)) {
console.error(
`${this._name}.currentPageLabel: "${val}" is not a valid page.`);
}
} }
/** /**
@ -279,7 +289,7 @@ class BaseViewer {
*/ */
set currentScale(val) { set currentScale(val) {
if (isNaN(val)) { if (isNaN(val)) {
throw new Error('Invalid numeric scale'); throw new Error('Invalid numeric scale.');
} }
if (!this.pdfDocument) { if (!this.pdfDocument) {
return; return;
@ -668,8 +678,8 @@ class BaseViewer {
const pageView = (Number.isInteger(pageNumber) && const pageView = (Number.isInteger(pageNumber) &&
this._pages[pageNumber - 1]); this._pages[pageNumber - 1]);
if (!pageView) { if (!pageView) {
console.error( console.error(`${this._name}.scrollPageIntoView: ` +
`${this._name}.scrollPageIntoView: Invalid "pageNumber" parameter.`); `"${pageNumber}" is not a valid pageNumber parameter.`);
return; return;
} }