Closed Bug 1193903 Opened 6 years ago Closed 6 years ago

contentAreaUtils saveDocument/saveBrowser doesn't work with Print Preview documents

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
firefox43 --- fixed

People

(Reporter: jld, Assigned: jld)

References

Details

(Keywords: regression)

Attachments

(2 files)

As reported in bug 1101100 comment #41, the changes to e10s-enable nsWebBrowserPersist broke saving Print Preview documents.

Specifically: the Print Preview document doesn't have a page descriptor, and the contentAreaUtils code for getting the postdata and cache key would catch and ignore the exception from that, but the C++ version added in bug 1101100 does not.  So even in the non-e10s case, passing a Print Preview document (or similar) to |saveDocument| would throw an exception when it reads the |cacheKey| or |postData| properties.

The e10s case is worse, because WebBrowserPersistDocumentChild reads all the attributes on actor creation to eagerly send them up to the parent (to avoid sync IPC in the getters), so startPersistence will fail and we don't even get to |saveDocument|.  And |saveBrowser| doesn't give an onError callback to startPersistence, because I thought there was nothing useful I could do there, which isn't quite right — there isn't too much useful information at that point, besides a too-generic nsresult, but throwing a Components.Exception would at least make it obvious that *something* failed.
(These patches don't need to be in order, but this part is easier to test if there's still a bug to cause things to fail.)
Attachment #8647281 - Flags: review?(mconley)
Comment on attachment 8647281 [details] [diff] [review]
Part 1: Improve error reporting for out-of-process saveBrowser().

Review of attachment 8647281 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/content/contentAreaUtils.js
@@ +141,5 @@
>    persistable.startPersistence({
>      onDocumentReady: function (document) {
>        saveDocument(document, aSkipPrompt);
> +    },
> +    onError: function (status) {

Ah heh - I think we're going to bitrot each other. :) If you beat me to landing, I'll just go with your approach here - seems more robust.
Attachment #8647281 - Flags: review?(mconley) → review+
Comment on attachment 8647282 [details] [diff] [review]
Part 2: Fix nsWebBrowserPersist.

Review of attachment 8647282 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks jld!
Attachment #8647282 - Flags: review?(mconley) → review+
https://hg.mozilla.org/mozilla-central/rev/6c4e80bdc918
https://hg.mozilla.org/mozilla-central/rev/4ac646590af1
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
As promised, I have re-tested my Print Edit add-on with Nightly 43.0a1 (2015-08-15) (which includes this patch) in e10s and non-e10s windows.  Saving the print preview DOM works fine for both 'Web Page, complete' and 'Web Page, HTML only'.  Great!
You need to log in before you can comment on or make changes to this bug.