Closed
Bug 1388903
Opened 7 years ago
Closed 7 years ago
64-bit Firefox on Windows - NPN_GetValue() passes an invalid HWND to PrintDlg()
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(firefox-esr52 wontfix, firefox55 wontfix, firefox56 fixed, firefox57 fixed)
RESOLVED
FIXED
mozilla57
People
(Reporter: jeclark, Assigned: handyman)
References
Details
(Keywords: 64bit, flashplayer, regression)
Attachments
(3 files)
39.49 KB,
application/zip
|
Details | |
2.19 KB,
patch
|
jimm
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
4.85 KB,
patch
|
jimm
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Method: Unzip the the attached project. With Flash Player installed and enabled, load bin-debug/PrintTest.html in Firefox Click anywhere on the stage (the block of text) Result: A print dialogue does not appear. Expected: A print dialogue should appear. Developer Notes: Last known working build from firefox is https://download-origin.cdn.mozilla.net/pub/firefox/nightly/2017/01/2017-01-16-13-31-20-mozilla-release-l10n/firefox-51.0.en-GB.win64.installer.exe It is 64bit only and regression in FF side. The reason is that the PrintDlg() (Win32 API) fails with error (0xFFFF), which means an "Invalid Handle" is passed to PrintDlg(). The player in 64bit FF passes a parent HWND, which it got from FF through NPN_GetValue(), to PrintDlg() as it is running in windowless mode. We need help from Mozilla to figure out why PrintDlg fails with the HWND supplied by FF.
Updated•7 years ago
|
Comment 1•7 years ago
|
||
Probably sandboxing, although we have been touching printing recently as well.
Flags: needinfo?(jmathies)
Comment 2•7 years ago
|
||
Hey David, please investigate. Looks like this might be flash sandbox related.
Flags: needinfo?(jmathies) → needinfo?(davidp99)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → davidp99
Assignee | ||
Comment 3•7 years ago
|
||
The issue is indeed the PrintDlg attempt to parent to the browser window. The sandbox won't allow that. Options: * Brokering PrintDlg in another process. I've done this locally using the brokering mechanism I'm putting together for bug 1382251. The principle issue here is that PrintDlg takes a callback that obviously can't be marshaled. I've run things without the callback and it seems to work ok. The callback is probably just used for styling. We could ask Adobe to confirm. We can lose styling if needed. * "Unparenting" the window by hijacking PrintDlg with the DLL interceptor and zeroing out the HWND Adobe gave us. This actually also works. We'll go this way short-term. Obviously, the print dialog can end up behind the browser window but that's manageable. This solution will fall down as we harden the NPAPI sandbox -- for example, it will never make it past Alternative Desktop. With the code in bug 1382251, both solutions are pretty simple. They result in the ability to print. However, a separate bug should be made for print-to-file if it isn't remedied by this bug, which it looks like it wont be. I'm not sure what API it uses for file access (apparently not GetSaveFileName, which would work even with file access sandboxed). I'll hack up a version (sans the intercepting code in bug 1382251) soon. The plan is to watch it for regressions for a week or so and then uplift.
Flags: needinfo?(davidp99)
Assignee | ||
Comment 4•7 years ago
|
||
This is the test for the DLL interceptor. No new opcodes were needed but I'll feel better about this when we've been in nightly for a bit without another version of the DLL inspiring crashes (not that this has happened before...). The next patch has the functionality. The architecture is limited by the quirks, which are set in 64-bit. This patch leaves an issue with Print to File. I'm not clear on why that is but I have confirmed that Print to file uses GetSaveFileNameW, which normally does permit file writing. I've filed that as bug 1395321. Tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9e763c81a51de0b7c8d091ff699932484968e21
Attachment #8902892 -
Flags: review?(jmathies)
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8902895 -
Flags: review?(jmathies)
Updated•7 years ago
|
Attachment #8902892 -
Flags: review?(jmathies) → review+
Updated•7 years ago
|
Attachment #8902895 -
Flags: review?(jmathies) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/56c962b8c853 Part 1: Add PrintDlgW to x64 DLL Interceptor Test. r=jimm https://hg.mozilla.org/integration/mozilla-inbound/rev/ad7fe0c5020c Part 2: Remove PrintDlg Window Parentage in NPAPI Process. r=jimm
Keywords: checkin-needed
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/56c962b8c853 https://hg.mozilla.org/mozilla-central/rev/ad7fe0c5020c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 8•7 years ago
|
||
Please request Beta approval on this when you're comfortable doing so.
status-firefox55:
--- → wontfix
status-firefox56:
--- → affected
status-firefox-esr52:
--- → wontfix
Flags: needinfo?(davidp99)
Flags: in-testsuite+
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8902892 [details] [diff] [review] 1. Adding PrintDlgW to x64 DLL Interceptor Test Approval Request Comment [Feature/Bug causing the regression]: Turning on NPAPI sandbox [User impact if declined]: Printing from plugins is completely broken. [Is this code covered by automated tests?]: The DLLInterceptor hook of PringDlgW is tested in Part 1. The actual dialog window and printing are not tested. [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No. [Why is the change risky/not risky?]: The change is very unlikely to lead to a crash or any inconsistency beyond the inability to print, which is already this bug. DLL Interceptor failures are always a possibility but they should lead to doing nothing. Also, the DLL interceptor portion is covered by tests. [String changes made/needed]: N/A
Flags: needinfo?(davidp99)
Attachment #8902892 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 10•7 years ago
|
||
Comment on attachment 8902895 [details] [diff] [review] 2. Remove PrintDlg Window Parentage in Sandboxed NPAPI Process Approval Request Comment [Feature/Bug causing the regression]: Turning on NPAPI sandbox [User impact if declined]: Printing from plugins is completely broken. [Is this code covered by automated tests?]: The DLLInterceptor hook of PringDlgW is tested in Part 1. The actual dialog window and printing are not tested. [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No. [Why is the change risky/not risky?]: The change is very unlikely to lead to a crash or any inconsistency beyond the inability to print, which is already this bug. DLL Interceptor failures are always a possibility but they should lead to doing nothing. Also, the DLL interceptor portion is covered by tests. [String changes made/needed]: N/A
Attachment #8902895 -
Flags: approval-mozilla-beta?
Comment 11•7 years ago
|
||
Comment on attachment 8902892 [details] [diff] [review] 1. Adding PrintDlgW to x64 DLL Interceptor Test Fix a regression and include a test. Beta56+.
Attachment #8902892 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•7 years ago
|
Attachment #8902895 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/9c1450703b0a https://hg.mozilla.org/releases/mozilla-beta/rev/755dff996043
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•