Skip to content

Commit

Permalink
Fixing collation support in Chrome OS printing
Browse files Browse the repository at this point in the history
This change updates the chosen attribute used to represent collation to
be the "multiple-document-handling" attribute in favor of
"sheet-collate". As stated in the PWG 5100.19 (IPP Implementor's Guide)
the "multiple-document-handling" attribute has the same semantics as
"sheet-collate" when printing multiple copies of a single document.

This change also modifies the print preview UI code to only display the
"collate" checkbox when the option is actually supported by the printer.

Bug: chromium:999527
Change-Id: I5bb5997a37ef5f14c33cd86b275a4ee9dbe03975
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2091718
Commit-Queue: David Valleau <valleau@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Auto-Submit: David Valleau <valleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752254}
  • Loading branch information
DavieV authored and Commit Bot committed Mar 21, 2020
1 parent 5873b49 commit 995f9ff
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
input-valid="{{inputValid_}}"
hint-message="[[getHintMessage_(copiesMax_)]]">
<div slot="opt-inside-content" class="checkbox" aria-live="polite"
hidden$="[[collateHidden_(currentValue_, inputValid_)]]">
hidden$="[[collateHidden_(
currentValue_, settings.collate.available, inputValid_)]]">
<cr-checkbox id="collate" on-change="onCollateChange_"
disabled="[[disabled]]" aria-labelledby="copies-collate-label">
<span id="copies-collate-label">$i18n{optionCollate}</span>
Expand Down
8 changes: 3 additions & 5 deletions chrome/browser/resources/print_preview/ui/copies_settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ Polymer({
disabled: Boolean,
},

observers: [
'onSettingsChanged_(settings.copies.value, settings.collate.value)',
],
observers: ['onSettingsChanged_(settings.copies.value, settings.collate.*)'],

/**
* @return {number} The maximum number of copies this printer supports.
Expand Down Expand Up @@ -99,8 +97,8 @@ Polymer({
* @private
*/
collateHidden_() {
return !this.inputValid_ || this.currentValue_ === '' ||
parseInt(this.currentValue_, 10) === 1;
return !this.getSetting('collate').available || !this.inputValid_ ||
this.currentValue_ === '' || parseInt(this.currentValue_, 10) === 1;
},

/** @private */
Expand Down
11 changes: 10 additions & 1 deletion chrome/test/data/webui/print_preview/copies_settings_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,15 @@ suite('CopiesSettingsTest', function() {
/** @type {?PrintPreviewCopiesSettingsElement} */
let copiesSection = null;

/** @type {?PrintPreviewModelElement} */
let model = null;

/** @override */
setup(function() {
PolymerTest.clearBody();
const model = document.createElement('print-preview-model');
model = document.createElement('print-preview-model');
document.body.appendChild(model);
model.set('settings.collate.available', true);

copiesSection = document.createElement('print-preview-copies-settings');
copiesSection.settings = model.settings;
Expand Down Expand Up @@ -68,6 +72,11 @@ suite('CopiesSettingsTest', function() {
copiesSection.setSetting('copies', 2);
assertFalse(collateSection.hidden);

model.set('settings.collate.available', false);
assertTrue(collateSection.hidden);
model.set('settings.collate.available', true);
assertFalse(collateSection.hidden);

// Set copies empty.
const copiesInput =
copiesSection.$$('print-preview-number-settings-section').getInput();
Expand Down
6 changes: 3 additions & 3 deletions printing/backend/cups_ipp_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

namespace printing {

constexpr char kIppCollate[] = "sheet-collate"; // RFC 3381
constexpr char kIppCollate[] = "multiple-document-handling"; // PWG 5100.19
constexpr char kIppCopies[] = CUPS_COPIES;
constexpr char kIppColor[] = CUPS_PRINT_COLOR_MODE;
constexpr char kIppMedia[] = CUPS_MEDIA;
Expand All @@ -19,8 +19,8 @@ constexpr char kIppPin[] = "job-password"; // PWG 5100.11
constexpr char kIppPinEncryption[] = "job-password-encryption"; // PWG 5100.11

// collation values
constexpr char kCollated[] = "collated";
constexpr char kUncollated[] = "uncollated";
constexpr char kCollated[] = "separate-documents-collated-copies";
constexpr char kUncollated[] = "separate-documents-uncollated-copies";

#if defined(OS_CHROMEOS)

Expand Down
3 changes: 2 additions & 1 deletion printing/backend/cups_ipp_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ PrinterSemanticCapsAndDefaults::Papers SupportedPapers(
bool CollateCapable(const CupsOptionProvider& printer) {
std::vector<base::StringPiece> values =
printer.GetSupportedOptionValueStrings(kIppCollate);
return base::Contains(values, kCollated);
return base::Contains(values, kCollated) &&
base::Contains(values, kUncollated);
}

bool CollateDefault(const CupsOptionProvider& printer) {
Expand Down

0 comments on commit 995f9ff

Please sign in to comment.