Closed
Bug 1238964
Opened 9 years ago
Closed 8 years ago
Printing via parent doesn't handle paper sizes correctly.
Categories
(Core :: Security: Process Sandboxing, defect)
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: bobowen, Assigned: bobowen)
References
Details
Attachments
(2 files, 1 obsolete file)
20.91 KB,
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
46.06 KB,
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
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•9 years ago
|
Summary: Printing via parent doesn't handling paper sizes correctly. → Printing via parent doesn't handle paper sizes correctly.
Assignee | ||
Comment 1•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=047ffd2e8afc
Assignee | ||
Comment 2•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4be8264a1c9c
Assignee | ||
Comment 3•8 years ago
|
||
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•8 years ago
|
||
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•8 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•8 years ago
|
||
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•8 years ago
|
Attachment #8709037 -
Attachment is obsolete: true
Attachment #8709037 -
Flags: review?(roc)
Comment 7•8 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•8 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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7d43e96b14c https://hg.mozilla.org/integration/mozilla-inbound/rev/fb4737474a9c
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e7d43e96b14c https://hg.mozilla.org/mozilla-central/rev/fb4737474a9c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment 11•8 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 | ||
Comment 12•8 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.
You need to log in
before you can comment on or make changes to this bug.
Description
•