Printing #252
Printing #252
Conversation
|
Works well in Firefox Desktop. You might want to check if |
I would like to do this, but
have you tested this? Because window.print is there on Firefox and Chrome android, it just doesn't do anything (https://crbug.com/322303). Also, we probably can't tell if printing is available at all (https://addons.mozilla.org/mobile/addon/cloud-printer/). So maybe we should rename the button to "Print view" or something on non-desktop? |
|
What about: http://stackoverflow.com/a/9273097 |
|
Well, you have to call print first before you can know whether it worked. So we could point out that they have to select print from their browser menu, but we don't even know if it's there. |
My idea is that we use the current implementation, and then alert the user and close the popup if the print fails (instead of doing nothing). |
|
But there are also browsers where |
|
Hmm, maybe we could include that in the alert. It could say, "We cannot print this file automatically. Please select 'Print' from inside your browser if it is available." |
|
Done. Could you add both l10n ids? Also, this probably requires some more thinking about security than I did (postMessage, untrusted documents). |
|
I will add the IDs. What security concerns did you have? |
| }); | ||
| reader.readAsText(new Blob([ | ||
| [ | ||
| '<!DOCTYPE html>', |
There was a problem hiding this comment.
This does not block merging, but is there a way that we could put this in an HTML file?
There was a problem hiding this comment.
It's possible, but you would probably need similar tricks to those in #246 to get it working in Firefox (not that that's bad, maybe we could unify the document displaying of both places).
As I said, I haven't thought about it at all, but basically:
|
IIRC @jackd1 did some sort of workaround for this in the module loader. Jack, could you clarify? |
|
I used XHR to get the html, then replaced all occurrences of [ORIGIN_OF_MAIN_DOCUMENT] in all of the scripts with the actual origin, basically like an include of sorts. Failing to replace it would result in the script not being authenticated aND refusing communication with the parent. |
|
There's another call to postMessage * in this PR though, although it doesn't seem like a problem to me. That leaves sandboxing. There is CSP sandbox, which is identical to iframe sandbox, but Firefox doesn't support it (https://bugzil.la/671389). Another possibility is maybe somehow getting the window to be a different origin than the opener, which (I think) happens if the opener doesn't have an origin, so that could work. |
I tried this and not surprisingly it doesn't work, probably for the same https://bugzil.la/1095808 mentioned in the bug for this (#206 (comment)). My only idea left is manually sanitizing the document, I'll file a bug for that. |
|
I've changed my mind, among other reasons because https://bugzil.la/1095808 seems to be specific to printing to a file. The document to be printed is now sandboxed, for details see commit message. |
|
That looks like a good solution 👍 |
Currently scripts in documents are allowed, so we need to sandbox them so they can't go and read other documents. We can't put the document in a sandboxed iframe because that breaks clean pagination and reflow. Currently, meta sandbox is poorly supported, as are Service Workers (which would also be a heavyweight solution). So we open the window from a sandboxed iframe, because no-allow-same- origin trickles down to the opened window. We then position that iframe over the print button, because of popup blockers.
WIP, because mobile UX could be better. In particular, if someone wants to test Chrome iOS that would be much appreciated.