Printing via parent doesn't handle paper sizes correctly.

RESOLVED FIXED in Firefox 46

Status

()

Core
Security: Process Sandboxing
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bobowen, Assigned: bobowen)

Tracking

46 Branch
mozilla46
All
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
This was found when I landed bug 1156742, so printing via parent had to be disabled.

The paper size wasn't getting passed down to the content process as I thought.
By chance the profiles I'd used for testing worked.

I've broken this out into a separate bug so I can land the fix separately for testing first.
Then I'll re-land the default print via parent pref in bug 115742, which got reopened.

I have a patch, but I want to do some extra tidying in some related code as well.
(Assignee)

Updated

2 years ago
Summary: Printing via parent doesn't handling paper sizes correctly. → Printing via parent doesn't handle paper sizes correctly.
(Assignee)

Comment 1

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=047ffd2e8afc
(Assignee)

Comment 2

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4be8264a1c9c
(Assignee)

Comment 3

2 years ago
Created attachment 8709035 [details] [diff] [review]
Part 1: Hold new printable page sizes in print nsIPrintSettingsWin

This also holds the resolution in the print settings, so that we can start to
remove the access to the native Windows print devices in the child process.
Attachment #8709035 - Flags: review?(roc)
(Assignee)

Comment 4

2 years ago
Created attachment 8709037 [details] [diff] [review]
Part 2: Move separate DEVMODE to nsIPrintSettings copying into nsPrintSettingsWin

This also corrects what I think are long standing issues with the mapping to and
from print settings on Windows.
Firstly it only uses the DEVMODE flags to decide what should get stored, in the
old code if paper size was not set, it would then use that possibly invalid
paper size to map to length and width. Paper setting prefs are mapped back if
they were stored or if they have been manually set to something sane.
Secondly it corrects the calculation that was used to convert from millimeters
to tenths of millimeters.
It also gets rid of the paperSizeType field, which was only used on Windows and
doesn't actually make sense according to the DEVMODE documentation as the
dmPaperLength and dmPaperWidth fields override the dmPaperSize, but can in
theory be specified at the same time.
Attachment #8709037 - Flags: review?(roc)
(Assignee)

Comment 5

2 years ago
Comment on attachment 8709035 [details] [diff] [review]
Part 1: Hold new printable page sizes in print nsIPrintSettingsWin

Sorry roc, hadn't noticed you were out ... jimm's graciously stepped in. :-)
Attachment #8709035 - Flags: review?(roc) → review?(jmathies)
(Assignee)

Comment 6

2 years ago
Created attachment 8709502 [details] [diff] [review]
Part 2: Move separate DEVMODE to nsIPrintSettings copying into nsPrintSettingsWin

This also corrects what I think are long standing issues with the mapping to and
from print settings on Windows.
Firstly it only uses the DEVMODE flags to decide what should get stored, in the
old code if paper size was not set, it would then use that possibly invalid
paper size to map to length and width. Paper setting prefs are mapped back if
they were stored or if they have been manually set to something sane.
Secondly it corrects the calculation that was used to convert from millimeters
to tenths of millimeters.
It also gets rid of the paperSizeType field, which was only used on Windows and
doesn't actually make sense according to the DEVMODE documentation as the
dmPaperLength and dmPaperWidth fields override the dmPaperSize, but can in
theory be specified at the same time.
Attachment #8709502 - Flags: review?(jmathies)
(Assignee)

Updated

2 years ago
Attachment #8709037 - Attachment is obsolete: true
Attachment #8709037 - Flags: review?(roc)

Comment 7

2 years ago
Comment on attachment 8709035 [details] [diff] [review]
Part 1: Hold new printable page sizes in print nsIPrintSettingsWin

Review of attachment 8709035 [details] [diff] [review]:
-----------------------------------------------------------------

::: embedding/components/printingui/win/nsPrintDialogUtil.cpp
@@ +985,5 @@
>      // Unlock DeviceNames
>      ::GlobalUnlock(prntdlg.hDevNames);
>  
>      // Transfer the settings from the native data to the PrintSettings
>      LPDEVMODEW devMode = (LPDEVMODEW)::GlobalLock(prntdlg.hDevMode);

would you mind filing a 'good first bug' on creating a mozilla version of ScopedHGlobal and using it in this print code?
Attachment #8709035 - Flags: review?(jmathies) → review+

Comment 8

2 years ago
Comment on attachment 8709502 [details] [diff] [review]
Part 2: Move separate DEVMODE to nsIPrintSettings copying into nsPrintSettingsWin

Review of attachment 8709502 [details] [diff] [review]:
-----------------------------------------------------------------

nice.
Attachment #8709502 - Flags: review?(jmathies) → review+

Comment 9

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7d43e96b14c
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb4737474a9c

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e7d43e96b14c
https://hg.mozilla.org/mozilla-central/rev/fb4737474a9c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox46: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
(Assignee)

Updated

2 years ago
Blocks: 1242456

Comment 11

2 years ago
These changes causes build failure with my environment(Win7 64bit, VS2015 64bit).

widget/windows/nsDeviceContextSpecWin.cpp(477): error C2664: 'HDC CreateICW(LPCWSTR,LPCWSTR,LPCWSTR,const DEVMODEW *)': cannot convert argument 2 from 'const char16_t *' to 'LPCWSTR'
(Assignee)

Updated

2 years ago
Depends on: 1242952
(Assignee)

Comment 12

2 years ago
(In reply to Toshihiro Yamada from comment #11)
> These changes causes build failure with my environment(Win7 64bit, VS2015
> 64bit).
> 
> widget/windows/nsDeviceContextSpecWin.cpp(477): error C2664: 'HDC
> CreateICW(LPCWSTR,LPCWSTR,LPCWSTR,const DEVMODEW *)': cannot convert
> argument 2 from 'const char16_t *' to 'LPCWSTR'

Thanks, I've filed bug 1242952 and will fix as soon as I can.
Depends on: 1242295
(Assignee)

Updated

2 years ago
Depends on: 1257825
(Assignee)

Updated

2 years ago
Depends on: 1260413

Updated

2 years ago
Depends on: 1271348
(Assignee)

Updated

2 years ago
Depends on: 1271900

Updated

2 years ago
Depends on: 1280159
You need to log in before you can comment on or make changes to this bug.