window.close is not waiting window print dialog
Categories
(Core :: Printing: Output, defect, P2)
Tracking
()
People
(Reporter: jorgefms, Assigned: emilio)
References
Details
(Whiteboard: [print2020_v81])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:81.0) Gecko/20100101 Firefox/81.0
Steps to reproduce:
newWin=window.open();
newWin.focus();
newWin.print();
newWin.close();
Actual results:
The page closes without waiting the print dialog box.
Expected results:
The page should closes only after click on print dialog box to print or cancel, like in all the old print preview.
With print.tab_modal.enabled set to False it works ok.
Comment 1•4 years ago
|
||
Bugbug thinks this bug should belong to this component, but please revert this change in case of error.
Assignee | ||
Comment 2•4 years ago
|
||
Can you attach a test-case to the bug? I think this should be fixed on Nightly via bug 1636728, but want to confirm.
Updated•4 years ago
|
Comment 4•4 years ago
|
||
OK, so I can repro by just running those 4 lines from comment #0 in a web console for e.g. this bugzilla page.
The difference between the broken and working case is that in nsGlobalWindowOuter::PrintOuter
at https://searchfox.org/mozilla-central/rev/2b250967a66886398e5e798371484fd018d88a22/dom/base/nsGlobalWindowOuter.cpp#5268-5269 , we determine that we're going to "print preview", and so in Print
on the same class, for this code:
if (aIsPreview) {
aError = webBrowserPrint->PrintPreview(aPrintSettings, aListener,
std::move(aPrintPreviewCallback));
} else {
// Historically we've eaten this error.
webBrowserPrint->Print(aPrintSettings, aListener);
}
we take the aIsPreview == true
path, which doesn't block.
So obvious options I see include:
- make the printpreview call block (maybe just here) or block with a spin loop or something (ew.)
- prevent website-initiated window closure if we hit this path. It looks like
nsDocumentViewer::RequestWindowClose
already has some logic to check for a pending print job, and defer the close until the print job is finished if so, which presumably we could build on?
So I suspect we should go for 2 - Emilio / :jwatt, any gut sense of how best to orchestrate that? We should already be able to determine whether the window is in modal state, but I doubt we want to block window closing at all times for that reason (e.g. would want users to be able to close the tab!). I don't know this code well enough to know if there's already something more specific exposed on the window/docshell/browsingcontext/contentviewer that we can use, or if we need to introduce a new bool or something.
Updated•4 years ago
|
Reporter | ||
Comment 5•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)
Can you attach a test-case to the bug? I think this should be fixed on Nightly via bug 1636728, but want to confirm.
Just run these 4 lines in console:
newWin=window.open();
newWin.focus();
newWin.print();
newWin.close();
Test with print.tab_modal.enabled TRUE and FALSE to see the difference
Assignee | ||
Comment 6•4 years ago
|
||
Yeah, we probably need to make window.print() block.
Comment 7•4 years ago
|
||
As a note, window.print() isn't specified as blocking or not, and Safari has it non-blocking…
Assignee | ||
Comment 8•4 years ago
|
||
As much as I hate nested event loops, I think we need to do it for this
case :)
Hopefully print() already assumes nested event loops can happen because
of the native dialog so it's already properly set up for this.
Assignee | ||
Comment 9•4 years ago
|
||
(In reply to Blake Winton (:bwinton) (:☕️) from comment #7)
As a note, window.print() isn't specified as blocking or not, and Safari has it non-blocking…
So safari behaves like us on the test-case above? I don't recall that behavior when testing window.print on WebKit... But I can double-check I guess
Comment 10•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #9)
(In reply to Blake Winton (:bwinton) (:☕️) from comment #7)
As a note, window.print() isn't specified as blocking or not, and Safari has it non-blocking…
So safari behaves like us on the test-case above? I don't recall that behavior when testing window.print on WebKit... But I can double-check I guess
It blocks in Safari Version 13.1.2 (15609.3.5.1.3) on macOS, despite what MDN says ( https://developer.mozilla.org/en-US/docs/Web/API/Window/print )
Comment 11•4 years ago
|
||
Comment 12•4 years ago
|
||
Comment on attachment 9173868 [details]
Bug 1662975 - Don't return from window.print() until the print dialog is closed. r=jwatt,nika
Beta/Release Uplift Approval Request
- User impact if declined: Site initiated ("Print" button) printing can be broken.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky):
- String changes made/needed:
Comment 13•4 years ago
|
||
Comment on attachment 9173868 [details]
Bug 1662975 - Don't return from window.print() until the print dialog is closed. r=jwatt,nika
Approved for 81.0b6. Fingers crossed I guess.
Comment 14•4 years ago
|
||
bugherder uplift |
Comment 15•4 years ago
|
||
Backed out changeset 52d0f9a901a9 (bug 1662975) for nested_fullscreen.https.html failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/aabe6661eb34db4590e7159dd3f3825d26c071db
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314826366&repo=autoland&lineNumber=7969
[task 2020-09-03T20:34:26.308Z] 20:34:26 INFO - TEST-START | /webxr/dom-overlay/nested_fullscreen.https.html
[task 2020-09-03T20:34:26.308Z] 20:34:26 INFO - Closing window 40
[task 2020-09-03T20:34:51.323Z] 20:34:51 INFO - Got timeout in harness
[task 2020-09-03T20:34:51.323Z] 20:34:51 INFO - TEST-UNEXPECTED-TIMEOUT | /webxr/dom-overlay/nested_fullscreen.https.html | TestRunner hit external timeout (this may indicate a hang)
[task 2020-09-03T20:34:51.323Z] 20:34:51 INFO - TEST-INFO took 25038ms
[task 2020-09-03T20:34:51.333Z] 20:34:51 INFO - Restarting browser for new test group
[task 2020-09-03T20:36:05.729Z] 20:36:05 INFO - Browser exited with return code -15
[task 2020-09-03T20:36:05.729Z] 20:36:05 INFO - PROCESS LEAKS None
[task 2020-09-03T20:36:05.729Z] 20:36:05 INFO - Browser not responding, setting status to CRASH
[task 2020-09-03T20:36:05.729Z] 20:36:05 WARNING - Command left in command_queue during cleanup: 'test_ended', (<wptrunner.wpttest.TestharnessTest /webxr/dom-overlay/nested_fullscreen.https.html>, (<wptrunner.wpttest.TestharnessResult CRASH>, []))
[task 2020-09-03T20:36:05.729Z] 20:36:05 INFO - Closing logging queue
[task 2020-09-03T20:36:05.729Z] 20:36:05 WARNING - Action click failed
[task 2020-09-03T20:36:05.729Z] 20:36:05 WARNING - Traceback (most recent call last):
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/base.py", line 790, in process_action
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - result = action_handler(payload)
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/actions.py", line 13, in __call__
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - self.protocol.click.element(element)
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 441, in element
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - return element.click()
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 257, in click
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - {"id": self.id})
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/decorators.py", line 36, in _
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - m._handle_socket_failure()
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 654, in _handle_socket_failure
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - reraise(exc_cls, exc, tb)
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/decorators.py", line 26, in _
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - return func(*args, **kwargs)
[task 2020-09-03T20:36:05.730Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 594, in _send_message
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - msg = self.client.request(name, params)
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/transport.py", line 276, in request
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - return self.receive()
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/transport.py", line 162, in receive
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - raise socket.error("No data received over socket")
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - error: No data received over socket
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING -
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - Traceback (most recent call last):
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 729, in run_func
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - self.result = True, self.func(self.protocol, self.url, self.timeout)
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 846, in do_testharness
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - done, rv = handler(result)
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/base.py", line 776, in __call__
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - return callback(url, payload)
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/base.py", line 797, in process_action
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - self._send_message("complete", "error")
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/base.py", line 807, in _send_message
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - self.protocol.testdriver.send_message(message_type, status, message=message)
[task 2020-09-03T20:36:05.731Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 473, in send_message
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - self.parent.base.execute_script("window.postMessage(%s, '*')" % json.dumps(obj))
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 87, in execute_script
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - return method(script, new_sandbox=False, sandbox=None)
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 1592, in execute_script
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - rv = self._send_message("WebDriver:ExecuteScript", body, key="value")
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/decorators.py", line 26, in _
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - return func(*args, **kwargs)
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - File "/Users/cltbld/tasks/task_1599163735/build/venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 591, in _send_message
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - raise errors.InvalidSessionIdException("Please start a session")
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING - InvalidSessionIdException: Please start a session
[task 2020-09-03T20:36:05.732Z] 20:36:05 WARNING -
[task 2020-09-03T20:36:05.732Z] 20:36:05 INFO - queue closed
[task 2020-09-03T20:36:05.758Z] 20:36:05 INFO - Application command: /Users/cltbld/tasks/task_1599163735/build/application/Firefox Nightly.app/Contents/MacOS/firefox --marionette about:blank -foreground -profile /var/folders/82/wd5jdtj95kq8kjnyxp1wjrhc000017/T/tmpHGX9sw
[task 2020-09-03T20:36:05.763Z] 20:36:05 INFO - Starting runner
[task 2020-09-03T20:36:08.178Z] 20:36:08 INFO - PID 1853 | console.error: SearchCache: "_readCacheFile: Error reading cache file:" (new Error("", "(unknown module)"))
[task 2020-09-03T20:36:09.399Z] 20:36:09 INFO - PID 1853 | 1599165369377 Marionette INFO Listening on port 64713
[task 2020-09-03T20:36:09.632Z] 20:36:09 INFO - TEST-START | /WebIDL/ecmascript-binding/legacy-platform-object/DefineOwnProperty.html
Comment 16•4 years ago
|
||
Bogdan, it looks like https://hg.mozilla.org/integration/autoland/rev/4519f0bb8624c6f1d4ee1c7a971a973d51f8193a which landed just after this bug's commit is the culprit of the failure.
Comment 17•4 years ago
|
||
Comment 19•4 years ago
|
||
No worries! Thanks!
Updated•4 years ago
|
Comment 20•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Comment 21•4 years ago
|
||
Aren't we creating any browser chrome tests for the new print feature? Given the importance of this bug I would have expected to see at least one landed together with the patch here.
Assignee | ||
Comment 22•4 years ago
|
||
Yes, there are. Our window.print() tests are pretty scarce though, and I was on a rush writing this yesterday (I was technically on PTO...). I'll write one though, leaving ni? around.
Updated•4 years ago
|
Comment 24•4 years ago
|
||
Reproduced this issue using Firefox 82.0a1 (BuildId:20200903151816).
This issue is verified fixed using Firefox 82.0a1 (BuildId:20200906215057) and Firefox 81.0b7 (BuildId:20200906164749) on Windows 10 64bit, macOS 10.14 & Ubuntu 18.04 64bit.
Updated•4 years ago
|
Comment 26•4 years ago
|
||
We are still having this bug with Firefox 82.0b3 and 83.0a1 (2020-09-24)
Comment 27•4 years ago
|
||
Here is a small test case we used on success with Firefox 80, but not with 81 and further builds :
<!DOCTYPE html>
<html>
<body>
Hello, World !
<script type="text/javascript">
window.print();
console.log('Test');
window.close();
</script>
</html>
Assignee | ||
Comment 28•4 years ago
|
||
(In reply to ElDiablo from comment #27)
Ah, so that's different. That's because you're calling window.print()
before onload fires, so we delay it, and we should be delaying window.close()
too, probably, instead of forgetting about it.
Your test-case does if you do something like this:
<!doctype html>
<html>
<body>
Hello, World !
<script type="text/javascript">
onload = function() {
setTimeout(function() {
window.print();
console.log('Test');
window.close();
}, 0);
};
</script>
</html>
Will file a separate bug for this.
Updated•3 years ago
|
Description
•