New issue
Advanced search Search tips

Issue 819806 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Print Preview custom margins cannot be set to decimal values from text boxes

Project Member Reported by rbpotter@chromium.org, Mar 7 2018

Issue description

Chrome Version: 64.0.3282.186, 65.0.3325.146
OS: Linux & Win10. Have not tried others.

What steps will reproduce the problem?
(1) Open print preview on any page
(2) Select custom margins from dropdown.
(3) Click on text box for one of the margins and try to enter e.g. 0.5"

What is the expected result?
Margin is set to new value

What happens instead?
Margin textbox turns red indicating invalid input and margin does not change.

This must have worked at some point, need to test old builds to determine when it broke.
 
Owner: rbpotter@chromium.org
Status: Started (was: Untriaged)
Tracked this down to https://chromium-review.googlesource.com/c/chromium/src/+/696719 and have a fix in progress.
Thanks for tracking this. We should attempt to merge this to M66. WDYT?
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8058925cd10eafa4a1a605c7d50c9efb252cc6c

commit c8058925cd10eafa4a1a605c7d50c9efb252cc6c
Author: rbpotter <rbpotter@chromium.org>
Date: Fri Mar 09 18:19:46 2018

Print Preview: Fix decimal text box inputs for custom margins

Custom margins text boxes were incorrectly not accepting decimal
inputs, due to a regression in delimiter value computation. Fix
regression and add test.

Bug:  819806 
Change-Id: I73093bbd0193e388cfc29b92f5397e0b73a9f23d
Reviewed-on: https://chromium-review.googlesource.com/954711
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542151}
[modify] https://crrev.com/c8058925cd10eafa4a1a605c7d50c9efb252cc6c/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/c8058925cd10eafa4a1a605c7d50c9efb252cc6c/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc

Labels: Merge-Request-66 OS-Chrome OS-Mac
Verified this on latest Canary 67.0.3368.1 on Windows 10. Margins can be set to decimal values with text boxes. As suggested in comment 2, requesting a merge to M66 since this is a regression and the fix is very safe - only 1 character change + unit test addition.
Project Member

Comment 5 by sheriffbot@chromium.org, Mar 13 2018

Labels: -Merge-Request-66 Merge-Approved-66 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M66. Please go ahead and merge the CL to branch 3359 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 14 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79d23f6c61a935f956b6e856e973c6b9cb60df41

commit 79d23f6c61a935f956b6e856e973c6b9cb60df41
Author: rbpotter <rbpotter@chromium.org>
Date: Wed Mar 14 17:16:46 2018

Print Preview: Fix decimal text box inputs for custom margins (M66)

Custom margins text boxes were incorrectly not accepting decimal
inputs, due to a regression in delimiter value computation. Fix
regression and add test.

Bug:  819806 
Change-Id: I73093bbd0193e388cfc29b92f5397e0b73a9f23d
Reviewed-on: https://chromium-review.googlesource.com/954711
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542151}(cherry picked from commit c8058925cd10eafa4a1a605c7d50c9efb252cc6c)
TBR: dpapad@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/962941
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#234}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/79d23f6c61a935f956b6e856e973c6b9cb60df41/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/79d23f6c61a935f956b6e856e973c6b9cb60df41/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8d499774a7fb9d05fb0aa00c9d133b2eae253b9

commit b8d499774a7fb9d05fb0aa00c9d133b2eae253b9
Author: rbpotter <rbpotter@chromium.org>
Date: Thu Mar 15 19:31:06 2018

PrintPreviewHandler: Clean up unit test

Use ScopedRestoreDefaultLocale and correct variable name style.

Bug:  819806 
Change-Id: I51436ceba079c411c051c602bbf9a1d90b3bfc32
Reviewed-on: https://chromium-review.googlesource.com/963749
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543471}
[modify] https://crrev.com/b8d499774a7fb9d05fb0aa00c9d133b2eae253b9/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc

Sign in to add a comment