Page range comparisons should use document size.

Preview can update page count, so PageNumberSet may represent page ranges incorrectly. Now PageNumberSet is used as temporarily objects which never stored.

PreviewGenerator stores PageRange which can be compared with other independently of current document size.

Removed code duplication in page ranges parsing code and parse directly to page ranges.

BUG=173822

Review URL: https://chromiumcodereview.appspot.com/12209086

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182005 0039d316-1c4b-4281-b951-d872f2087c98
parent 8ade58a6
......@@ -20,7 +20,7 @@ cr.define('print_preview', function() {
* Number of pages in the document to print.
* @type {number}
*/
this.pageCount = 1;
this.pageCount = 0;
/**
* Size of the pages of the document in points.
......
......@@ -20,29 +20,6 @@ cr.define('print_preview', function() {
this.pageNumberSet_ = pageListToPageSet(pageNumberList);
};
/**
* @param {string} pageRangeStr String form of a page range. I.e. '2,3,4-5'.
* If string is empty, all page numbers will be in the page number set.
* @param {number} totalPageCount Total number of pages in the original
* document.
* @return {print_preview.PageNumberSet} Page number set parsed from the
* given page range string and total page count. Null returned if
* the given page range string is invalid.
*/
PageNumberSet.parse = function(pageRangeStr, totalPageCount) {
if (pageRangeStr == '') {
var pageNumberList = [];
for (var i = 0; i < totalPageCount; i++) {
pageNumberList.push(i + 1);
}
return new PageNumberSet(pageNumberList);
} else {
return isPageRangeTextValid(pageRangeStr, totalPageCount) ?
new PageNumberSet(
pageRangeTextToPageList(pageRangeStr, totalPageCount)) : null;
}
};
PageNumberSet.prototype = {
/** @return {number} The number of page numbers in the set. */
get size() {
......@@ -74,28 +51,10 @@ cr.define('print_preview', function() {
return this.pageNumberSet_.indexOf(pageNumber);
},
/**
* @return {!Array.<object.<{from: number, to: number}>>} A list of page
* ranges suitable for use in the native layer.
*/
getPageRanges: function() {
return pageSetToPageRanges(this.pageNumberSet_);
},
/** @return {!Array.<number>} Array representation of the set. */
asArray: function() {
return this.pageNumberSet_.slice(0);
},
/**
* @param {print_preview.PageNumberSet} other Page number set to compare
* against.
* @return {boolean} Whether another page number set is equal to this one.
*/
equals: function(other) {
return other == null ?
false : areArraysEqual(this.pageNumberSet_, other.pageNumberSet_);
}
};
// Export
......
......@@ -51,7 +51,7 @@ cr.define('print_preview', function() {
*/
this.documentInfo_ = new print_preview.DocumentInfo();
this.documentInfo_.isModifiable = true;
this.documentInfo_.pageCount = 1;
this.documentInfo_.pageCount = 0;
this.documentInfo_.pageSize = initialPageSize;
this.documentInfo_.printableArea = new print_preview.PrintableArea(
new print_preview.Coordinate2d(0, 0), initialPageSize);
......@@ -592,6 +592,23 @@ cr.define('print_preview', function() {
return this.pageRange_.getValue();
},
/**
* @return {!Array.<object.<{from: number, to: number}>>} Page ranges
* represented by of the page range string.
*/
getPageRanges: function() {
return this.pageRange_.getPageRanges();
},
/**
* @return {!Array.<object.<{from: number, to: number}>>} Page ranges
* represented by of the page range string and constraied by ducument
* page count.
*/
getDocumentPageRanges: function() {
return this.pageRange_.getDocumentPageRanges();
},
/**
* @return {!print_preview.PageNumberSet} Page number set specified by the
* string representation of the page range string.
......
......@@ -24,13 +24,21 @@ cr.define('print_preview.ticket_items', function() {
this.documentInfo_ = documentInfo;
};
/**
* Impossibly large page number.
* @type {number}
* @const
* @private
*/
PageRange.MAX_PAGE_NUMBER_ = 1000000000;
PageRange.prototype = {
__proto__: print_preview.ticket_items.TicketItem.prototype,
/** @override */
wouldValueBeValid: function(value) {
return value == '' ||
isPageRangeTextValid(value, this.documentInfo_.pageCount);
return null != pageRangeTextToPageRanges(value,
this.documentInfo_.pageCount);
},
/**
......@@ -38,13 +46,9 @@ cr.define('print_preview.ticket_items', function() {
* page range string.
*/
getPageNumberSet: function() {
if (this.isValid()) {
return print_preview.PageNumberSet.parse(
var pageNumberList = pageRangeTextToPageList(
this.getValue(), this.documentInfo_.pageCount);
} else {
return print_preview.PageNumberSet.parse(
this.getDefaultValueInternal(), this.documentInfo_.pageCount);
}
return new print_preview.PageNumberSet(pageNumberList);
},
/** @override */
......@@ -60,7 +64,28 @@ cr.define('print_preview.ticket_items', function() {
/** @override */
getCapabilityNotAvailableValueInternal: function() {
return '';
}
},
/**
* @return {!Array.<object.<{from: number, to: number}>>} A list of page
* ranges.
*/
getPageRanges: function() {
var page_ranges = pageRangeTextToPageRanges(this.getValue());
return page_ranges ? page_ranges : [];
},
/**
* @return {!Array.<object.<{from: number, to: number}>>} A list of page
* ranges suitable for use in the native layer.
* TODO(vitalybuka): this should be removed when native layer switched to
* page ranges.
*/
getDocumentPageRanges: function() {
var page_ranges = pageRangeTextToPageRanges(this.getValue(),
this.documentInfo_.pageCount);
return page_ranges ? page_ranges : [];
},
};
// Export
......
......@@ -140,12 +140,8 @@ cr.define('print_preview', function() {
assert(printTicketStore.isTicketValidForPreview(),
'Trying to generate preview when ticket is not valid');
var pageRanges =
(requestId > 0 && printTicketStore.hasPageRangeCapability()) ?
printTicketStore.getPageNumberSet().getPageRanges() : [];
var ticket = {
'pageRange': pageRanges,
'pageRange': printTicketStore.getDocumentPageRanges(),
'landscape': printTicketStore.isLandscapeEnabled(),
'color': printTicketStore.isColorEnabled() ?
NativeLayer.ColorMode_.COLOR : NativeLayer.ColorMode_.GRAY,
......@@ -215,8 +211,7 @@ cr.define('print_preview', function() {
'Trying to print when ticket is not valid');
var ticket = {
'pageRange': printTicketStore.hasPageRangeCapability() ?
printTicketStore.getPageNumberSet().getPageRanges() : [],
'pageRange': printTicketStore.getDocumentPageRanges(),
'pageCount': printTicketStore.getPageNumberSet().size,
'landscape': printTicketStore.isLandscapeEnabled(),
'color': printTicketStore.isColorEnabled() ?
......
......@@ -77,11 +77,11 @@ cr.define('print_preview', function() {
this.isFitToPageEnabled_ = false;
/**
* Page number set used to generate the last preview.
* @type {print_preview.PageNumberSet}
* Page ranges setting used used to generate the last preview.
* @type {!Array.<object.<{from: number, to: number}>>}
* @private
*/
this.pageNumberSet_ = null;
this.pageRanges_ = null;
/**
* Margins type used to generate the last preview.
......@@ -158,7 +158,7 @@ cr.define('print_preview', function() {
this.printTicketStore_.isHeaderFooterEnabled();
this.isColorEnabled_ = this.printTicketStore_.isColorEnabled();
this.isFitToPageEnabled_ = this.printTicketStore_.isFitToPageEnabled();
this.pageNumberSet_ = this.printTicketStore_.getPageNumberSet();
this.pageRanges_ = this.printTicketStore_.getPageRanges();
this.marginsType_ = this.printTicketStore_.getMarginsType();
this.isCssBackgroundEnabled_ =
this.printTicketStore_.isCssBackgroundEnabled();
......@@ -259,7 +259,8 @@ cr.define('print_preview', function() {
ticketStore.isHeaderFooterEnabled() != this.isHeaderFooterEnabled_ ||
ticketStore.isColorEnabled() != this.isColorEnabled_ ||
ticketStore.isFitToPageEnabled() != this.isFitToPageEnabled_ ||
!ticketStore.getPageNumberSet().equals(this.pageNumberSet_) ||
this.pageRanges_ == null ||
!areRangesEqual(ticketStore.getPageRanges(), this.pageRanges_) ||
(ticketStore.getMarginsType() != this.marginsType_ &&
ticketStore.getMarginsType() !=
print_preview.ticket_items.MarginsType.Value.CUSTOM) ||
......@@ -323,7 +324,7 @@ cr.define('print_preview', function() {
return; // Ignore old response.
}
this.printTicketStore_.updatePageCount(event.pageCount);
this.pageNumberSet_ = this.printTicketStore_.getPageNumberSet();
this.pageRanges_ = this.printTicketStore_.getPageRanges();
},
/**
......
......@@ -15,7 +15,6 @@ function isInteger(toTest) {
/**
* Returns true if |value| is a valid non zero positive integer.
* @param {string} value The string to be tested.
*
* @return {boolean} true if the |value| is valid non zero positive integer.
*/
function isPositiveInteger(value) {
......@@ -24,9 +23,8 @@ function isPositiveInteger(value) {
/**
* Returns true if the contents of the two arrays are equal.
* @param {Array} array1 The first array.
* @param {Array} array2 The second array.
*
* @param {Array.<{from: number, to: number}>} array1 The first array.
* @param {Array.<{from: number, to: number}>} array2 The second array.
* @return {boolean} true if the arrays are equal.
*/
function areArraysEqual(array1, array2) {
......@@ -38,6 +36,23 @@ function areArraysEqual(array1, array2) {
return true;
}
/**
* Returns true if the contents of the two page ranges are equal.
* @param {Array} array1 The first array.
* @param {Array} array2 The second array.
* @return {boolean} true if the arrays are equal.
*/
function areRangesEqual(array1, array2) {
if (array1.length != array2.length)
return false;
for (var i = 0; i < array1.length; i++)
if (array1[i].from != array2[i].from ||
array1[i].to != array2[i].to) {
return false;
}
return true;
}
/**
* Removes duplicate elements from |inArray| and returns a new array.
* |inArray| is not affected. It assumes that |inArray| is already sorted.
......@@ -58,94 +73,86 @@ function removeDuplicates(inArray) {
}
/**
* Checks if |pageRangeText| represents a valid page selection.
* Returns a list of ranges in |pageRangeText|. The ranges are
* listed in the order they appear in |pageRangeText| and duplicates are not
* eliminated. If |pageRangeText| is not valid null is returned.
* A valid selection has a parsable format and every page identifier is
* <= |totalPageCount| unless wildcards are used (see examples).
* greater the 0 and less or equal to |totalPageCount| unless wildcards are
* used(see examples).
* If |totalPageCount| is 0 or undefined function uses impossibly large number
* instead.
* Wildcard the first number must be larger then 0 and less or equal then
* |totalPageCount|. If it's missed then 1 is used as the first number.
* Wildcard the second number must be larger then the first number. If it's
* missed then |totalPageCount| is used as the second number.
* Example: "1-4, 9, 3-6, 10, 11" is valid, assuming |totalPageCount| >= 11.
* Example: "1-4, 6-6" is valid, assuming |totalPageCount| >= 6.
* Example: "1-4, -6" is valid, assuming |totalPageCount| >= 6.
* Example: "2-" is valid, assuming |totalPageCount| >= 2, means from 2 to the
* end.
* Example: "1-10000" is valid, regardless of |totalPageCount|, means from 1 to
* the end if |totalPageCount| < 10000.
* Example: "4-2, 11, -6" is invalid.
* Example: "-" is valid, assuming |totalPageCount| >= 1.
* Example: "1-4dsf, 11" is invalid regardless of |totalPageCount|.
* Example: "4-2, 11, -6" is invalid regardless of |totalPageCount|.
*
* Note: If |totalPageCount| is undefined the validation does not take
* |totalPageCount| into account.
* Example: "34853253" is valid.
* Example: "1-4, 9, 3-6, 10, 11" is valid.
*
* @param {string} pageRangeText The text to be checked.
* @param {number} totalPageCount The total number of pages.
* @return {boolean} true if the |pageRangeText| is valid.
* @return {Array.<{from: number, to: number}>} An array of page range objects.
*/
function isPageRangeTextValid(pageRangeText, totalPageCount) {
var regex = /^\s*([0-9]+)\s*-\s*([0-9]*)\s*$/;
var successfullyParsed = 0;
function pageRangeTextToPageRanges(pageRangeText, totalPageCount) {
var MAX_PAGE_NUMBER = 1000000000;
totalPageCount = totalPageCount ? totalPageCount : MAX_PAGE_NUMBER;
// Splitting around commas.
var regex = /^\s*([0-9]*)\s*-\s*([0-9]*)\s*$/;
var parts = pageRangeText.split(/,/);
var pageRanges = [];
for (var i = 0; i < parts.length; ++i) {
// Removing whitespace.
parts[i] = parts[i].replace(/\s*/g, '');
var match = parts[i].match(regex);
if (parts[i].length == 0)
continue;
if (match && match[1] && isPositiveInteger(match[1])) {
var from = parseInt(match[1], 10);
var to = isPositiveInteger(match[2]) ? parseInt(match[2], 10) :
totalPageCount;
if (match) {
if (!isPositiveInteger(match[1]) && match[1] !== '')
return null;
if (!isPositiveInteger(match[2]) && match[2] !== '')
return null;
var from = match[1] ? parseInt(match[1], 10) : 1;
var to = match[2] ? parseInt(match[2], 10) : totalPageCount;
if (from > to || from > totalPageCount)
return false;
} else if (!isPositiveInteger(parts[i]) || (totalPageCount != -1 &&
parseInt(parts[i], 10) > totalPageCount)) {
return false;
return null;
pageRanges.push({'from': from, 'to': to});
} else {
if (!isPositiveInteger(parts[i]))
return null;
var singlePageNumber = parseInt(parts[i], 10);
if (singlePageNumber > totalPageCount)
return null;
pageRanges.push({'from': singlePageNumber, 'to': singlePageNumber});
}
successfullyParsed++;
}
return successfullyParsed > 0;
return pageRanges;
}
/**
* Returns a list of all pages specified in |pagesRangeText|. The pages are
* Returns a list of pages defined by |pagesRangeText|. The pages are
* listed in the order they appear in |pageRangeText| and duplicates are not
* eliminated. If |pageRangeText| is not valid according to
* isPageRangeTextValid(), or |totalPageCount| is undefined an empty list is
* returned.
* eliminated. If |pageRangeText| is not valid according or
* |totalPageCount| undefined [1,2,...,totalPageCount] is returned.
* See pageRangeTextToPageRanges for details.
* @param {string} pageRangeText The text to be checked.
* @param {number} totalPageCount The total number of pages.
* @return {Array.<number>} A list of all pages.
*/
function pageRangeTextToPageList(pageRangeText, totalPageCount) {
var pageList = [];
if ((totalPageCount &&
!isPageRangeTextValid(pageRangeText, totalPageCount)) ||
!totalPageCount) {
return pageList;
}
var regex = /^\s*([0-9]+)\s*-\s*([0-9]*)\s*$/;
var parts = pageRangeText.split(/,/);
for (var i = 0; i < parts.length; ++i) {
var match = parts[i].match(regex);
if (match && match[1]) {
var from = parseInt(match[1], 10);
var to = match[2] ? parseInt(match[2], 10) : totalPageCount;
for (var j = from; j <= Math.min(to, totalPageCount); ++j)
var pageRanges = pageRangeTextToPageRanges(pageRangeText, totalPageCount);
pageList = [];
if (pageRanges) {
for (var i = 0; i < pageRanges.length; ++i) {
for (var j = pageRanges[i].from; j <= Math.min(pageRanges[i].to,
totalPageCount); ++j) {
pageList.push(j);
} else {
var singlePageNumber = parseInt(parts[i], 10);
if (isPositiveInteger(singlePageNumber.toString()) &&
singlePageNumber <= totalPageCount) {
pageList.push(singlePageNumber);
}
}
}
if (pageList.length == 0) {
for (var j = 1; j <= totalPageCount; ++j)
pageList.push(j);
}
return pageList;
}
......@@ -166,26 +173,6 @@ function pageListToPageSet(pageList) {
return pageSet;
}
/**
* Converts |pageSet| to page ranges. It squashes whenever possible.
* Example: '1-2,3,5-7' becomes 1-3,5-7.
*
* @param {Array.<number>} pageSet The set of pages to be processed. Callers
* should ensure that no duplicates exist.
* @return {Array.<{from: number, to: number}>} An array of page range objects.
*/
function pageSetToPageRanges(pageSet) {
var pageRanges = [];
for (var i = 0; i < pageSet.length; ++i) {
var tempFrom = pageSet[i];
while (i + 1 < pageSet.length && pageSet[i + 1] == pageSet[i] + 1)
++i;
var tempTo = pageSet[i];
pageRanges.push({'from': tempFrom, 'to': tempTo});
}
return pageRanges;
}
/**
* @param {!HTMLElement} element Element to check for visibility.
* @return {boolean} Whether the given element is visible.
......
......@@ -53,50 +53,65 @@ TEST_F('PrintPreviewUtilsUnitTest', 'RemoveDuplicates', function() {
assertTrue(areArraysEqual(removeDuplicates(array1), [1,2,3,6,7,9,10]));
});
TEST_F('PrintPreviewUtilsUnitTest', 'IsPageRangeTextValid1', function() {
var totalPageCount;
assertTrue(isPageRangeTextValid("1,2, 3,56,1000000", totalPageCount));
assertTrue(isPageRangeTextValid(", ,1,2,3,,1,, 56 ,", totalPageCount));
assertTrue(isPageRangeTextValid(",1-3,,6-9,6-10,", totalPageCount));
assertTrue(isPageRangeTextValid("10-", totalPageCount));
assertTrue(isPageRangeTextValid("10-10", totalPageCount));
assertTrue(isPageRangeTextValid(" 10-100000", totalPageCount));
assertFalse(isPageRangeTextValid("1,2,0,56,1000000", totalPageCount));
assertFalse(isPageRangeTextValid("-1,1,2,,56", totalPageCount));
assertFalse(isPageRangeTextValid("1,2,56-40", totalPageCount));
TEST_F('PrintPreviewUtilsUnitTest', 'PageRanges', function() {
function assertRangesEqual(simpleRange1, range2) {
var range1 = []
for (var i = 0; i < simpleRange1.length; i++) {
var from;
var to;
if (Array.isArray(simpleRange1[i])) {
from = simpleRange1[i][0];
to = simpleRange1[i][1];
} else {
from = simpleRange1[i];
to = simpleRange1[i];
}
range1.push({'from': from, 'to': to});
}
assertTrue(areRangesEqual(range1, range2));
};
assertRangesEqual([1, 2, 3, 1, 56],
pageRangeTextToPageRanges("1,2,3,1,56", 100));
assertRangesEqual([[1, 3],[6, 9], [6, 10]],
pageRangeTextToPageRanges("1-3, 6-9,6-10 ", 100));
assertRangesEqual([[10, 100]],
pageRangeTextToPageRanges("10-", 100));
assertRangesEqual([[10, 100000]],
pageRangeTextToPageRanges("10-100000", 100));
assertRangesEqual([[1, 100]],
pageRangeTextToPageRanges("-", 100));
assertRangesEqual([1, 2],
pageRangeTextToPageRanges("1,2", undefined));
assertRangesEqual([[1, 1000000000]],
pageRangeTextToPageRanges("-", null));
assertRangesEqual([[1, 1000000000]],
pageRangeTextToPageRanges("-", 0));
});
TEST_F('PrintPreviewUtilsUnitTest', 'IsPageRangeTextValid2', function() {
var totalPageCount = 100;
assertTrue(isPageRangeTextValid(",,1,2,3,,1,,56,", totalPageCount));
assertTrue(isPageRangeTextValid(",1-3,,6-9,6-10,", totalPageCount));
assertTrue(isPageRangeTextValid("10-", totalPageCount));
assertTrue(isPageRangeTextValid("10-100000", totalPageCount));
assertFalse(isPageRangeTextValid("1,2,3,56,1000000", totalPageCount));
assertFalse(isPageRangeTextValid("1,2,0,56", totalPageCount));
assertFalse(isPageRangeTextValid("-1,1,2,,56", totalPageCount));
assertFalse(isPageRangeTextValid("1,2,56-40", totalPageCount));
assertFalse(isPageRangeTextValid("101-110", totalPageCount));
TEST_F('PrintPreviewUtilsUnitTest', 'InvalidPageRanges', function() {
assertTrue(pageRangeTextToPageRanges("1,100000", 100) == null);
assertTrue(pageRangeTextToPageRanges("1,2,0,56", 100) == null);
assertTrue(pageRangeTextToPageRanges("-1,1,2,,56", 100) == null);
assertTrue(pageRangeTextToPageRanges("1,2,56-40", 100) == null);
assertTrue(pageRangeTextToPageRanges("101-110", 100) == null);
});
TEST_F('PrintPreviewUtilsUnitTest', 'PageRangeTextToPageList', function() {
var totalPageCount = 10;
assertTrue(areArraysEqual([1],
pageRangeTextToPageList("1", totalPageCount)));
pageRangeTextToPageList("1", 10)));
assertTrue(areArraysEqual([1,2,3,4],
pageRangeTextToPageList("1-4", totalPageCount)));
pageRangeTextToPageList("1-4", 10)));
assertTrue(areArraysEqual([1,2,3,4,2,3,4],
pageRangeTextToPageList("1-4, 2-4", totalPageCount)));
pageRangeTextToPageList("1-4, 2-4", 10)));
assertTrue(areArraysEqual([1,2,5,7,8,9,10,2,2,3],
pageRangeTextToPageList("1-2, 5, 7-10, 2, 2, 3",
totalPageCount)));
10)));
assertTrue(areArraysEqual([5,6,7,8,9,10],
pageRangeTextToPageList("5-", totalPageCount)));
assertTrue(areArraysEqual([], pageRangeTextToPageList("1-4", undefined)));
pageRangeTextToPageList("5-", 10)));
assertTrue(areArraysEqual([],
pageRangeTextToPageList("1-abcd", totalPageCount)));
pageRangeTextToPageList("1-4", undefined)));
assertTrue(areArraysEqual([1,2,3,4,5,6,7,8,9,10],
pageRangeTextToPageList("1-abcd", 10)));
});
TEST_F('PrintPreviewUtilsUnitTest', 'PageListToPageSet', function() {
......@@ -104,14 +119,3 @@ TEST_F('PrintPreviewUtilsUnitTest', 'PageListToPageSet', function() {
assertTrue(areArraysEqual([1,2,3,4], pageListToPageSet([1,2,2,3,4,1,1,1])));
assertTrue(areArraysEqual([], pageListToPageSet([])));
});
TEST_F('PrintPreviewUtilsUnitTest', 'PageSetToPageRanges', function() {
var pageRanges = pageSetToPageRanges([1,2,3,7,8,9,11]);
assertEquals(pageRanges.length, 3);
assertEquals(pageRanges[0].from, 1);
assertEquals(pageRanges[0].to, 3);
assertEquals(pageRanges[1].from, 7);
assertEquals(pageRanges[1].to, 9);
assertEquals(pageRanges[2].from, 11);
assertEquals(pageRanges[2].to, 11);
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment