Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2708213003:
Fix fit to page scaling display for landscape (Closed)
DescriptionFix fit to page scaling display for landscape
Found there is a bug in the scaling value displayed when "Fit to Page"
is checked for landscape oriented documents due to the difference in
orientation between printable area and the document. Correct for
orientation in the fit to page scaling computation so that the correct
value will be displayed in the UI.
BUG=695636
Review-Url: https://codereview.chromium.org/2708213003
Cr-Commit-Position: refs/heads/master@{#452685}
Committed: https://chromium.googlesource.com/chromium/src/+/83f9a7b7a698e9cb7b6e03c2ad567ab5d1435d93
Patch Set 1 #
Total comments: 2
Patch Set 2 : Simplify rotate check #Messages
Total messages: 24 (18 generated)
The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run
Description was changed from ========== Fix fit to page scaling display for landscape Found there is a bug in the scaling value displayed when "Fit to Page" is checked for landscape oriented documents due to the difference in orientation between printable area and the document. Correct for orientation in the fit to page scaling computation so that the correct value will be displayed in the UI. BUG=None ========== to ========== Fix fit to page scaling display for landscape Found there is a bug in the scaling value displayed when "Fit to Page" is checked for landscape oriented documents due to the difference in orientation between printable area and the document. Correct for orientation in the fit to page scaling computation so that the correct value will be displayed in the UI. BUG=None ==========
rbpotter@chromium.org changed reviewers: + thestig@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
https://codereview.chromium.org/2708213003/diff/1/components/printing/rendere... File components/printing/renderer/print_web_view_helper.cc (right): https://codereview.chromium.org/2708213003/diff/1/components/printing/rendere... components/printing/renderer/print_web_view_helper.cc:1264: bool rotate = ((printable_area_in_points.width() > Please double check and see if I got this right. I think it does the same thing but is a bit more readable. bool is_printable_area_landscape = printable_area_in_points.width() > printable_area_in_points.height(); bool is_preset_landscape = preset_options.uniformPageSize.width > preset_options.uniformPageSize.height; bool rotate = is_printable_area_landscape != is_preset_landscape;
The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix fit to page scaling display for landscape Found there is a bug in the scaling value displayed when "Fit to Page" is checked for landscape oriented documents due to the difference in orientation between printable area and the document. Correct for orientation in the fit to page scaling computation so that the correct value will be displayed in the UI. BUG=None ========== to ========== Fix fit to page scaling display for landscape Found there is a bug in the scaling value displayed when "Fit to Page" is checked for landscape oriented documents due to the difference in orientation between printable area and the document. Correct for orientation in the fit to page scaling computation so that the correct value will be displayed in the UI. BUG=695636 ==========
https://codereview.chromium.org/2708213003/diff/1/components/printing/rendere... File components/printing/renderer/print_web_view_helper.cc (right): https://codereview.chromium.org/2708213003/diff/1/components/printing/rendere... components/printing/renderer/print_web_view_helper.cc:1264: bool rotate = ((printable_area_in_points.width() > On 2017/02/23 01:32:38, Lei Zhang (super slow) wrote: > Please double check and see if I got this right. I think it does the same thing > but is a bit more readable. > > bool is_printable_area_landscape = printable_area_in_points.width() > > printable_area_in_points.height(); > bool is_preset_landscape = preset_options.uniformPageSize.width > > preset_options.uniformPageSize.height; > bool rotate = is_printable_area_landscape != is_preset_landscape; Looks like this works and it is indeed more readable. Thanks!
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by rbpotter@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487894847956710,
"parent_rev": "2e6a9ebbc75837e1f7986ecc7f16e5d59d70299a", "commit_rev":
"83f9a7b7a698e9cb7b6e03c2ad567ab5d1435d93"}
Message was sent while issue was closed.
Description was changed from ========== Fix fit to page scaling display for landscape Found there is a bug in the scaling value displayed when "Fit to Page" is checked for landscape oriented documents due to the difference in orientation between printable area and the document. Correct for orientation in the fit to page scaling computation so that the correct value will be displayed in the UI. BUG=695636 ========== to ========== Fix fit to page scaling display for landscape Found there is a bug in the scaling value displayed when "Fit to Page" is checked for landscape oriented documents due to the difference in orientation between printable area and the document. Correct for orientation in the fit to page scaling computation so that the correct value will be displayed in the UI. BUG=695636 Review-Url: https://codereview.chromium.org/2708213003 Cr-Commit-Position: refs/heads/master@{#452685} Committed: https://chromium.googlesource.com/chromium/src/+/83f9a7b7a698e9cb7b6e03c2ad56... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/83f9a7b7a698e9cb7b6e03c2ad56... |
