Regression: [Print Preview]Customized margin lines doesn't stay on their position.
Reported by
dchau...@etouch.net,
Aug 2
|
||
Issue descriptionChrome Version: 70.0.3510.2 (Official Build) Revision 19bbe7b51081dd5aca29d10d5f881cce7eb95ff8-refs/branch-heads/3510@{#4} 32/64-bit. OS: Win(7,8,8.1,10) Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14) and Linux(14.04 LTS). Precondition : Enable #enable-new-print-preview flag from chrome://flags What steps will reproduce the problem? 1. Launch Chrome, give print command on any webpage and click on 'More settings' button. 2. Select 'Custom' option from 'Margins' drop-down list and customize the margin lines on preview page. 3. Now open the 'Margins' drop-down list and select any other option (For Ex: Default). 4. Again select 'Custom' option from 'Margins' drop-down list and observe the margin lines on preview page. Actual: Customized margin lines doesn't stay on there position on changing the option from 'Margin' drop-down list. Expected: Customized margin line should stay on there position on changing the option from Margin drop-down list. This is a regression issue, broken in M-67 series, below is manual regression range: Good build: 67.0.3383.0 (Revision: 546672) Bad build: 67.0.3385.0 (Revision: 547349) Using the per-revision bisect providing the bisect results: You are probably looking for a change made after 547257 (known good), but no later than 547259 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/d71dca6d566f55004c7af6a4f4b18651287932ab..55cbc9d778e123931b1a2901a07e25a4950f8a31 Suspecting: https://chromium.googlesource.com/chromium/src/+/55cbc9d778e123931b1a2901a07e25a4950f8a31 @rbpotter: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. NOTE: This issue is also reproducible on Stable #68.0.3440.84, Beta #68.0.3440.75 and Dev #69.0.3497.23 Kindly review the attached screen-cast for reference. Thank you.
,
Aug 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f commit 59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f Author: rbpotter <rbpotter@chromium.org> Date: Fri Aug 17 20:05:06 2018 Print Preview: Fix custom margins logic and add tests Correct Print Preview custom margin logic to match the old UI. The following rules should be followed: 1) If there is no sticky custom margins value, when custom margins are selected, they should be initialized to the document margins to avoid a preview regeneration. 2) If there is a sticky value, custom margins should be initialized to that sticky value, even if this results in a regeneration of the preview. 3) If at any point the user changes the orientation or the media size, the sticky custom margins value is cleared. This happens even if the media size is changed while the margins are not available, i.e., while previewing a PDF, because the media size value is sticky. Bug: 870249 Change-Id: Ibbe5084e34a8b80ec452ec9697184f0c274d2132 Reviewed-on: https://chromium-review.googlesource.com/1161288 Reviewed-by: Scott Chen <scottchen@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/heads/master@{#584163} [modify] https://crrev.com/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f/chrome/browser/resources/print_preview/new/BUILD.gn [modify] https://crrev.com/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f/chrome/browser/resources/print_preview/new/margin_control_container.html [modify] https://crrev.com/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f/chrome/browser/resources/print_preview/new/margin_control_container.js [modify] https://crrev.com/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f/chrome/browser/resources/print_preview/new/model.js [modify] https://crrev.com/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f/chrome/browser/resources/print_preview/new/preview_area.html [modify] https://crrev.com/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f/chrome/browser/resources/print_preview/new/preview_area.js [modify] https://crrev.com/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f/chrome/browser/resources/print_preview/new/settings_select.js [modify] https://crrev.com/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f/chrome/test/data/webui/print_preview/custom_margins_test.js [modify] https://crrev.com/59e9c4bcb836b6d0dcb3174259d6a7ec700c2c6f/chrome/test/data/webui/print_preview/new_print_preview_ui_browsertest.js
,
Sep 26
Should be fixed by the CL in comment 2, which also added automated tests to verify. |
||
►
Sign in to add a comment |
||
Comment 1 by rbpotter@chromium.org
, Aug 14