DocumentPropertiesW's return values are handled incorrectly in CreateGlobalDevModeAndInit

RESOLVED FIXED in Firefox 51

Status

()

Core
Printing: Setup
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: so61pi, Unassigned)

Tracking

unspecified
mozilla51
All
Windows
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160407164938
(Reporter)

Comment 1

2 years ago
DocumentPropertiesW's return type is LONG and negative value indicates an error. But currently in CreateGlobalDevModeAndInit it is casted to DWORD which can change its sign-ness.
OS: Unspecified → All
Hardware: Unspecified → All
(Reporter)

Updated

2 years ago
OS: All → Windows

Comment 2

2 years ago
Thanks.

This should probably be in Printing: Setup ... my fault I filed the previous one in the wrong component.
Status: UNCONFIRMED → NEW
Component: Printing: Output → Printing: Setup
Ever confirmed: true
Comment hidden (mozreview-request)

Comment 4

2 years ago
mozreview-review
Comment on attachment 8783403 [details]
Bug 1265739 - DocumenetProperties should use LONG as return type.

https://reviewboard.mozilla.org/r/73202/#review71426

::: embedding/components/printingui/win/nsPrintDialogUtil.cpp:484
(Diff revision 1)
>  
>    // Make sure hPrinter is closed on all paths
>    nsAutoPrinter autoPrinter(hPrinter);
>  
>    // Get the buffer size
> -  DWORD dwNeeded = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, nullptr,
> +  LONG needed = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, nullptr,

nit - indentation

::: embedding/components/printingui/win/nsPrintDialogUtil.cpp:503
(Diff revision 1)
>    nsAutoGlobalMem globalDevMode(hDevMode);
>    if (!hDevMode) {
>      return nsReturnRef<nsHGLOBAL>();
>    }
>  
> -  DWORD dwRet = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, newDevMode,
> +  LONG ret = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, newDevMode,

nit - indentation

::: embedding/components/printingui/win/nsPrintDialogUtil.cpp:523
(Diff revision 1)
>    nsCOMPtr<nsIPrintSettingsWin> psWin = do_QueryInterface(aPS);
>    MOZ_ASSERT(psWin);
>    psWin->CopyToNative(devMode);
>  
>    // Sets back the changes we made to the DevMode into the Printer Driver
> -  dwRet = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, devMode, devMode,
> +  ret = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, devMode, devMode,

nit - indentation

::: widget/windows/nsDeviceContextSpecWin.cpp:346
(Diff revision 1)
>  
>      // Allocate a buffer of the correct size.
> -    dwNeeded = ::DocumentPropertiesW(nullptr, hPrinter, name, nullptr, nullptr, 0);
> +    LONG needed = ::DocumentPropertiesW(nullptr, hPrinter, name, nullptr,
> +                                        nullptr, 0);
> +    if (needed < 0) {
> +      PR_PL(("**** nsDeviceContextSpecWin::GetDataFromPrinter - Couldn't get size of DEVMODE using DocumentPropertiesW(pDeviceName = \"%s\"). GetLastEror() = %08x\n",

lets wrap this sting over so it's not over 80 char limit.

Comment 5

2 years ago
mozreview-review
Comment on attachment 8783403 [details]
Bug 1265739 - DocumenetProperties should use LONG as return type.

https://reviewboard.mozilla.org/r/73202/#review71430
Attachment #8783403 - Flags: review?(jmathies) → review+

Comment 6

2 years ago
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/mozilla-inbound/rev/79e57fc02263
DocumenetProperties should use LONG as return type. r=jimm

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/79e57fc02263
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.