Skip to content

Commit

Permalink
Drop borderless sizes from media size list
Browse files Browse the repository at this point in the history
The paper size dropdown only allows one entry for any given size.  When
a borderless and non-borderless version of the same size are available,
only one of them will make it into the list.  In the past the
non-borderless size generally happened to be selected because the
printer would list the borderless version second.  With
crrev.com/c/4022966, Chrome started sorting the list into a consistent
order.  Depending on how the printer names its borderless sizes, it
could end up with things like "A 4.borderless" sorting ahead of "A4",
which causes the regular A4 size to disappear.

Since borderless sizes don't currently produce different output from
non-borderless sizes, let's remove the borderless sizes from the list
entirely.  This ensures that the "regular" sizes are always selected.
We don't currently have a good way to detect all the borderless patterns
without just dropping non-standard paper names entirely, but the two
patterns added here cover all of the problem patterns that were observed
in our test lab.

(cherry picked from commit 49d1fac)

Bug: b/263452851
Test: Manually compared paper size lists on lab printers
Change-Id: Ie34f104a3c3f285c836eb6fdb0382931df749eeb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4157931
Commit-Queue: Benjamin Gordon <bmgordon@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1091614}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4163339
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#259}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
yetamrra authored and Chromium LUCI CQ committed Jan 13, 2023
1 parent df96524 commit 3e33978
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
8 changes: 8 additions & 0 deletions chrome/common/printing/print_media_l10n.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <map>
#include <string>

#include "base/containers/contains.h"
#include "base/i18n/string_compare.h"
#include "base/no_destructor.h"
#include "base/strings/string_piece.h"
Expand Down Expand Up @@ -699,6 +700,13 @@ void SortPaperDisplayNames(std::vector<PaperWithSizeInfo>& papers) {

// Break apart the list into separate sort groups.
for (auto& p : papers) {
// Drop borderless sizes so they don't get sorted ahead of standard sizes.
// TODO(b/218752273): Remove once borderless sizes are handled properly.
if (base::Contains(p.paper.vendor_id, ".borderless_") ||
base::Contains(p.paper.vendor_id, ".fb_")) {
continue;
}

switch (p.size_info.sort_group) {
case MediaSizeGroup::kSizeMm:
mm_sizes.emplace_back(p);
Expand Down
20 changes: 20 additions & 0 deletions chrome/common/printing/print_media_l10n_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -192,4 +192,24 @@ TEST(PrintMediaL10N, SortNamedSizes) {
}
}

TEST(PrintMediaL10N, RemoveBorderlessSizes) {
PaperWithSizeInfo p1 = {MediaSizeInfo{u"AAA", MediaSizeGroup::kSizeNamed},
Paper{"AAA", "oe_aaa.fb_8x10in", gfx::Size(8, 10)}};
PaperWithSizeInfo p2 = {MediaSizeInfo{u"BBB", MediaSizeGroup::kSizeNamed},
Paper{"BBB", "oe_bbb_4x6in", gfx::Size(4, 6)}};
PaperWithSizeInfo p3 = {
MediaSizeInfo{u"BBB", MediaSizeGroup::kSizeNamed},
Paper{"BBB", "oe_bbb.borderless_4x6in", gfx::Size(4, 6)}};
PaperWithSizeInfo p4 = {MediaSizeInfo{u"AAA", MediaSizeGroup::kSizeNamed},
Paper{"AAA", "oe_aaa.8x10in", gfx::Size(8, 10)}};

std::vector<PaperWithSizeInfo> papers = {p1, p2, p3, p4};
std::vector<PaperWithSizeInfo> expected = {p4, p2};
SortPaperDisplayNames(papers);
ASSERT_EQ(papers.size(), expected.size());
for (size_t i = 0; i < expected.size(); i++) {
VerifyPaperSizeMatch(papers[i], expected[i]);
}
}

} // namespace printing

0 comments on commit 3e33978

Please sign in to comment.