Closed Bug 1379117 Opened 3 years ago Closed 2 years ago
Preview should only be allowed on ns IWeb Browser Print when retrieved through the docshell .
Once we get rid of non-WebExtensions addons we should be able to turn the warning at  into a full assertion. We generally only hit this when we have retrieved the nsIWebBrowserPrint interface through nsGlobalWindow and not nsDocShell::GetPrintPreview. It's possible that this is currently used by addons, but not after Fx57. There are at least two tests that also do this: layout/base/tests/chrome/test_printpreview_bug396024.xul layout/base/tests/chrome/test_printpreview_bug482976.xul So, these should be changed to use the docshell or otherwise rewritten.  https://hg.mozilla.org/mozilla-central/file/a0b5515b13eb/layout/base/nsDocumentViewer.cpp#l4050
Any plans to do this now, Bob?
Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
Priority: P3 → P1
Changing the last retrieval of the nsIWebBrowserPrint in printpreview_bug396024_helper.xul caused problems because doing it via the docshell interrupts the load that triggers run5. So, I moved the test to run5, which is more consistent with run2 and run3. I then realised that the frames would have switched anyway, so I changed it to retrieve frame for the last test, which I think makes more sense. It's not totally clear to me what this was testing originally or whether it continued to do so after the addition of the second iframe a long time ago.
Attachment #8949118 - Flags: review?(jwatt)
I noticed these while looking for other things that were gettting an nsIWebBrowserPrint for print preview. I think these must have been left for legacy addons and they don't seem to be used, so I think they can go.
Attachment #8949119 - Flags: review?(jwatt)
Comment on attachment 8949118 [details] [diff] [review] Part 1: MOZ_ASSERT when not retrieving nsIWebBrowserPrint through docshell for Print Preview Review of attachment 8949118 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsDocumentViewer.cpp @@ +3980,1 @@ > "Using docshell.printPreview is the preferred way for print previewing!"); Seems like "preferred" may not be the most appropriate term any more if we're now MOZ_ASSERT'ing.
Attachment #8949118 - Flags: review?(jwatt) → review+
(In reply to Jonathan Watt [:jwatt] (needinfo? me) from comment #5) ... > > "Using docshell.printPreview is the preferred way for print previewing!"); > > Seems like "preferred" may not be the most appropriate term any more if > we're now MOZ_ASSERT'ing. Good point, thanks. Changed to: "For print preview nsIWebBrowserPrint must be from docshell.printPreview!" Also, pushed again to try, because I picked up some other thing that seemed to be perma-failing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=17cd0c71ae746bc6b8acf37be95717885046eaa6
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/806f9f8166be Part 1: MOZ_ASSERT when not retrieving nsIWebBrowserPrint through docshell for Print Preview. r=jwatt https://hg.mozilla.org/integration/mozilla-inbound/rev/58996b7409fc Part 2: Remove some deprecated PrintUtils functions. r=jwatt
You need to log in before you can comment on or make changes to this bug.