Closed
Bug 487667
Opened 15 years ago
Closed 15 years ago
Clone documents for printing
Categories
(Core :: Print Preview, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: smaug, Assigned: smaug)
References
(Blocks 2 open bugs)
Details
Attachments
(4 files, 14 obsolete files)
340 bytes,
text/html
|
Details | |
1.30 KB,
text/html
|
Details | |
195.98 KB,
patch
|
joe
:
review+
|
Details | Diff | Splinter Review |
126.53 KB,
patch
|
Details | Diff | Splinter Review |
So we want to clone documents for printing. There are few things to think about, for example 1) How to handle XBL 2) How to clone stylesheets/rules 3) and how to clone document, but use the cloned stylesheets/rules ...
Assignee | ||
Comment 1•15 years ago
|
||
I think we could ignore XBL until XBL2 arrives at least. I can't imagine people seriously use XBL with printed documents. When cloning, how about serializing all style sheets and inline styles to inline <style> blocks?
I agree regarding XBL. It'll mean that <video>s won't have controls, and <marquee>s won't scroll, but in both cases that sounds like what we want anyway. Doesn't the CSS code have a copy-on-write powers already? Can we use those?
Comment 4•15 years ago
|
||
What's the concern with cloning stylesheets, exactly?
Comment 5•15 years ago
|
||
And to be clear, I'm not quite sure what the second paragraph of comment 2 is suggesting...
I was just suggesting that if it's difficult to clone stylesheets and ensure the clones are properly attached to the cloned document, we could serialize the stylesheets of the original document and paste those serializations as inline style sheets in the cloned document.
Comment 7•15 years ago
|
||
I don't think there should be any issues with cloning sheets. We do it all the type for chrome, for example: all the chrome sheets are clones of the sheets living in the prototype cache.
Assignee | ||
Comment 8•15 years ago
|
||
(In reply to comment #4) > What's the concern with cloning stylesheets, exactly? Maybe nothing. I found some usable API after filing this bug :) Going to test how it works... 4) who should own the cloned document, 5) and should there be a separate docshell for it, 6) or is re-using document viewer ok. (though global object handling might get tricky)
Assignee | ||
Comment 9•15 years ago
|
||
For print preview docshell could have a method like cloneAndLoadAsPrintPreview(nsIDOMDocument* aDoc) That would initialize things like for normal pages, but just use the cloned document and its stylesheets. That way, depending on the UI requirements, print preview could be opened in a new tab or window. Well, even using the same tab should work, but that would push the original page to bfcache (if bfcachable). I'm not sure about printing, but I think document viewer could handle that, pretty much like it is handled now.
The cloned document should behave much like an external document IMHO. That is, it should have its own documentviewer but no associated docshell. The cloned document will never be viewed directly. After it has reflowed, the print engine can just print it. For print preview I want to create new XUL API that lets chrome create "page elements" and render the print document into them. This will let chrome and themes control the look and behaviour of the decorations around each page and other aspects of print preview. For example, chrome could display pages "N-up", allow the user to visually select pages to print, etc. This approach means that layout will no longer need to distinguish between "print" and "print preview" presentations. Some code can be removed from layout. We can be 100% sure that the print layout matches the print preview layout exactly, since they're the same layout. We no longer have to worry about the pesky "print preview" scrollbar. The strange print preview frame tree (where there's one nsViewportFrame at the root and each page is *also* an nsViewportFrame in the same frame tree) will go away. For the XUL API, we'll need a way to query the print document for the number of pages it has. To render the pages, I think the best approach would be to create a <portal> element with a setPage(doc,pageNumber) method. Its paint method would adjust the clipping and transform of the passed-in gfxContext and pass it off to the print document for painting. But I think the first step should be to get the cloned document approach working with printing.
Assignee | ||
Comment 11•15 years ago
|
||
Doesn't yet take care of iframes/frames nor catalogsheets etc. Removes the textfragment cloning.
Assignee | ||
Comment 12•15 years ago
|
||
..and script handling is a nice hack. Will replace with something else.
Handling plugins will be interesting. One approach would be to render the plugin into a canvas and replace the plugin with the canvas in the cloned document. This should work for windowless plugins but will probably fail hopelessly on windowed Linux and Windows plugins. Then again, I'm not sure printing works for them anyway.
Comment 14•15 years ago
|
||
We used to have cases where plugins generated their own postscript, at least on Linux. Has that gotten removed anyway with the various linux printing and cairo work in the 1.9 timeframe? That is, some plugins (java comes to mind) basically did vector, not bitmap, printing. The canvas approach would be a regression from that.
Assignee | ||
Comment 15•15 years ago
|
||
iframes/frames working, using an ugly hack. Will cleanup while trying to get <img> working.
Assignee | ||
Comment 16•15 years ago
|
||
So as a first step I'm aiming to provide similar printing behavior what we have now, but using cloned documents. Already that allows us to replace nsTObserverArray<nsIPresShell*> with nsIPresShell* in nsIDocument and nsPresShellIterator.h can be removed. And if wanted, nsIContent objects could have a pointer to their primary frame (if we really want that). nsIContent could probably have nsIFrame* member variable which is set lazily when primary frame is first time needed. And when destroying an nsIFrame, that could clear the relevant nsIContent::mFrame.
Comment 17•15 years ago
|
||
That last bit wouldn't work well given our current <area> mess, so let's not worry about it quite yet.
(In reply to comment #14) > We used to have cases where plugins generated their own postscript, at least on > Linux. Has that gotten removed anyway with the various linux printing and > cairo work in the 1.9 timeframe? Yes.
(In reply to comment #16) > And if wanted, nsIContent objects could have a pointer to their primary frame > (if we really want that). nsIContent could probably have nsIFrame* member > variable which is set lazily when primary frame is first time needed. There's no reason to not set it eagerly as far as I know.
Assignee | ||
Comment 20•15 years ago
|
||
This contains printfs and leaks and is full of hacks, but printing images/form elements/frames etc work. Apparently printing plugins doesn't work on OSX, so have to wait until I have access to linux again.
Attachment #372035 -
Attachment is obsolete: true
Comment 21•15 years ago
|
||
I've spun off the second half of comment 16 into bug 500882.
Blocks: 500882
Assignee | ||
Comment 22•15 years ago
|
||
Uploading the patch so that I have the backup. Printing <video> doesn't work yet (it doesn't work in 1.9.1/trunk, well, not in any reasonable way). Plugin printing isn't there yet either - it doesn't work on my main dev. environment, linux, anyway. Frame printing should work, and selection/multiple selections printing. I'm not quite happy how print preview setup works. It basically overrides the current presentation of some content viewer. Probably the presentation should be moved to cache and a new content viewer should be created for the print preview. For printing an artificial docshell is created. It is basically just a container for the document tree. Misses automated tests. Will update the patch and ask reviews hopefully on Monday or so.
Assignee: nobody → Olli.Pettay
Attachment #375093 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Comment 23•15 years ago
|
||
Do you need the artificial docshell? Can you get away with just a document viewer?
Assignee | ||
Comment 24•15 years ago
|
||
The docshell makes the setup pretty easy. (I)Frames add their docshell to the parent docshell and print engine can then iterate them and so on.
(In reply to comment #22) > I'm not quite happy how print preview setup works. It basically overrides the > current presentation of some content viewer. Probably the presentation should > be moved to cache and a new content viewer should be created for the print > preview. The master plan makes the print preview presentation go away anyway, so don't worry about it.
Assignee | ||
Comment 26•15 years ago
|
||
(In reply to comment #22) > Will update the patch and ask reviews hopefully on Monday or so. Bah. <video> is easy, but plugins are not, at least not for someone not too familiar with our gfx :(
Can't you call nsObjectFrame::PaintPlugin directly or indirectly, passing in a context pointing to a temporary surface? That should paint the plugin if we possibly can.
Comment 28•15 years ago
|
||
Some plugins expect to be able to do high resolution printing, Java in particular from what I hear. I know very little about how that's done on either end, but I hear they do vector drawing to somewhere for print output, printing to a lowres bitmap isn't good enough for them I don't think. Hao, do you know anything about what the Java plugin does for printing?
You mean on Windows? Perhaps we can capture the print output to a Windows metafile and play it back...
Comment 30•15 years ago
|
||
Yeah, on Windows. And that sounds like what they likely do today IIRC. No idea what happens on other platforms...
Assignee | ||
Comment 31•15 years ago
|
||
Contains fix for Bug 514487. External documents (svg effects etc) don't work, not sure how important and this shouldn't be too difficult to fix. Otherwise works pretty well, IMO. I'd like to make the docshell setup a bit better still. And more automatic tests. Currently we have tests only for very few cases like form control handling. - Plugin printing works like today. - image animation as well, though without the bugs which cause animations to restart even if the image is being printed/previewed. - SMIL animations aren't started when printing. (so at least this doesn't have the current "paint something random depending on the invalidation" behavior) - <video> prints the current frame or poster image. (So the current random-painting bug is fixed.) - selection printing works - (i)frame printing works, also when the document is included using <object> - internal svg effects work - styles are cloned Am I missing still something crucial?
Attachment #387958 -
Attachment is obsolete: true
Assignee | ||
Comment 32•15 years ago
|
||
Fixes a small problem in media element handling and adds a helper method to ensure reasonable document viewer for print preview.
Attachment #399186 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Attachment #399221 -
Flags: review?(roc)
Assignee | ||
Comment 33•15 years ago
|
||
Comment on attachment 399221 [details] [diff] [review] v15 Asking first a sort of "what you think about this" review :)
Assignee | ||
Comment 34•15 years ago
|
||
Attachment #399221 -
Attachment is obsolete: true
Attachment #399229 -
Flags: review?(roc)
Attachment #399221 -
Flags: review?(roc)
This is awesome. + PRBool IsPrintDocument() { return mIsPrintDocument; } + + virtual already_AddRefed<nsIDocument> + CloneForPrint(nsISupports* aCloneContainer); + + nsIDocument* GetOriginalDocument() { return mOriginalDocument; } We'll need documentation for these. + PRPackedBool mIsPrintDocument; + + PRPackedBool mCloningForPrint; + + nsCOMPtr<nsIDocument> mOriginalDocument; And these. mCloningForPrint is kind of nasty, why can't we have a CloneNodeInternal which takes it as a parameter? Or do the "if (mCloningForPrint)" stuff in CloneForPrint after we've cloned the document? + * Returns the nsIObjectFrame object which should be used when + * printing the plugin. + */ + [noscript] nsIFrame getPrintFrame(); It's not clear from the comment what this means or why you'd use it. + if (!doc || !(doc->IsPrintDocument() || mOwnerContent->IsInDoc())) { Unnecessary parens Is there a reason we don't do the stylesheet cloning in nsIDocument::CloneForPrint for all cloned documents? Similar question for nsHTMLOptionElement and nsHTMLTextAreaElement, can't we just always clone the selection and the value? What about the input values for other input types? Can you write a test for the ID-registration in nsNodeUtils::CloneAndAdopt? I guess that's an existing bug that script could encounter. + nsIFrame* nsiframe = do_QueryFrame(frame); + aDest->mPrintFrame = nsiframe; nsiframe isn't a good name, I'd just call it 'f' This way of handling plugins isn't ideal --- I really do think we should snapshot the plugin's current pixels if we can --- but I guess this is good enough for now. + if (doc && (aForceInDataDoc || + (!doc->IsLoadedAsData() || doc->IsPrintDocument()))) { Unnecessary parens + test_bug514487.html \ Looks like this file is missing? How can we test image cloning, video cloning, etc? I wonder if we can just clone the document for printing into an iframe which is the same size as the viewport and use that for the test, somehow. + virtual PRBool IsInitializedForPrintPreview() = 0; + + virtual void InitializeForPrintPreview() = 0; + + virtual void SetPrintPreviewPresentation(nsIWidget* aWidget, + nsIViewManager* aViewManager, + nsPresContext* aPresContext, Better document these + if (IsDynamic()) { + return; + } else { + ++gPrintImageRefs->Elements()[index]; + } Avoid else after return nsPresContext::ResetPrintImageAnimation is yucky. Could we avoid this by having nsImageLoadingContent::CloneImageForPrint replace the image request with a snapshot of the current frame of the image? It would be really nice if we could just remove nsPresContext::IsDynamic completely... I'm not sure what the changes in nsVideoFrame.cpp are... - po->mContent = aContent; Where did this go? I have some great ideas about improving the printing of selection by removing everything but the selected content from the cloned doc --- which would let us get rid of the "print selection" hacks we currently have in layout --- but that can wait I guess.
Assignee | ||
Comment 36•15 years ago
|
||
(In reply to comment #35) > mCloningForPrint is kind of nasty, why can't we have a CloneNodeInternal which > takes it as a parameter? Or do the "if (mCloningForPrint)" stuff in > CloneForPrint after we've cloned the document? I don't like mCloningForPrint either, but it was the simplest way to achieve whatever I needed. It is needed that some things happens before child nodes of the document are cloned. I still wanted to use the normal cloneNode method, so that I get all the functionality nsNodeUtils::CloneAndAdopt provides. > > + * Returns the nsIObjectFrame object which should be used when > + * printing the plugin. > + */ > + [noscript] nsIFrame getPrintFrame(); > > It's not clear from the comment what this means or why you'd use it. OK, will improve the comment. The idea is the same what we have now - use the original plugin instance for printing. > Is there a reason we don't do the stylesheet cloning in > nsIDocument::CloneForPrint for all cloned documents? Why should we clone stylesheets in normal case? > Similar question for nsHTMLOptionElement and nsHTMLTextAreaElement, can't we > just always clone the selection and the value? What about the input values for > other input types? Input elements do clone the values. These are question not strictly related to this bug, but to related to generic "what should be cloned when a node is cloned". > Can you write a test for the ID-registration in nsNodeUtils::CloneAndAdopt? I > guess that's an existing bug that script could encounter. That change is part of Bug 514487, and the patch contains few tests for it. > This way of handling plugins isn't ideal --- I really do think we should > snapshot the plugin's current pixels if we can --- but I guess this is good > enough for now. It spent quite a few days trying to figure out how to get a reasonable snapshot (never got it quite right), and even then I wasn't sure if that is the right thing - mainly because of comment 28. > + test_bug514487.html \ > > Looks like this file is missing? It is there > nsPresContext::ResetPrintImageAnimation is yucky. Could we avoid this by having > nsImageLoadingContent::CloneImageForPrint replace the image request with a > snapshot of the current frame of the image? The problem is that there are lots of other images than just <img>. All the borders and background etc and all those can be animated. At some point I did take the snapshot in CloneImageForPrint, but making that work nicely with other images was getting quite ugly. > I'm not sure what the changes in nsVideoFrame.cpp are... Hmm, windows newlines or something. I'll check that. > - po->mContent = aContent; > > Where did this go? nsPrintObject::Init
(In reply to comment #36) > (In reply to comment #35) > > mCloningForPrint is kind of nasty, why can't we have a CloneNodeInternal which > > takes it as a parameter? Or do the "if (mCloningForPrint)" stuff in > > CloneForPrint after we've cloned the document? > I don't like mCloningForPrint either, but it was the simplest way to achieve > whatever I needed. > It is needed that some things happens before child nodes of the document are > cloned. I still wanted to use the normal cloneNode method, so that > I get all the functionality nsNodeUtils::CloneAndAdopt provides. Why can't you pass 'aCloningForPrint' down through nsNodeUtils::Clone and nsNodeUtils::CloneAndAdopt? > > Is there a reason we don't do the stylesheet cloning in > > nsIDocument::CloneForPrint for all cloned documents? > Why should we clone stylesheets in normal case? It just seems to me that generally the clone should be as much like the original as possible. > > Similar question for nsHTMLOptionElement and nsHTMLTextAreaElement, can't we > > just always clone the selection and the value? What about the input values for > > other input types? > Input elements do clone the values. > These are question not strictly related to this bug, but to related to > generic "what should be cloned when a node is cloned". Same as above. I guess this is a spec question. > It spent quite a few days trying to figure out how to get a reasonable > snapshot (never got it quite right), and even then I wasn't sure if that > is the right thing - mainly because of comment 28. I think we could actually fix that, at least on Windows, by having plugins "print" on Windows to an enhanced metafile which we can then play back later. But let's deal with that elsewhere. > > nsPresContext::ResetPrintImageAnimation is yucky. Could we avoid this by having > > nsImageLoadingContent::CloneImageForPrint replace the image request with a > > snapshot of the current frame of the image? > The problem is that there are lots of other images than just <img>. > All the borders and background etc and all those can be animated. > At some point I did take the snapshot in CloneImageForPrint, but making > that work nicely with other images was getting quite ugly. I see. Hmm, that is hard. I guess one problem is that matching images in styles in the print presentation to images in styles in the main presentation is difficult, and maybe there isn't even a match due to media style rules. Right now you're actually showing the first frame of all style images, right? Let's suppose we carry on with that, so we only show the "current frame" for nsImageLoadingContent images. Why then do we need to manage style images via mPrintImages and gPrintImageContainers? Wouldn't it be enough to snapshot nsImageLoadingContent images and load all other images normally?
(In reply to comment #37) > Right now you're actually showing the first frame of all style images, right? As usual I'm confused. Correct me if I'm wrong again, but what's actually happening is that the style system's request for an image gets mapped to the existing image in the main presentation, and you're temporarily pausing the animation of that image, right? So we display the right frame of the image in the print presentation, but we stop the image animating in the main presentation... Then it seems we could fix this by adding code where we construct image requests. We want to be able to specify a flag which means "if there is an existing image for this URI, take a snapshot of its current frame and give me that snapshot instead of the real image" ... and pass that flag whenever we construct an image request for a print document.
Assignee | ||
Comment 39•15 years ago
|
||
(In reply to comment #38) > So we display the right frame of the image in > the print presentation, but we stop the image animating in the main > presentation... Right. So basically keeping our current behavior (with bug fixes).
True, but the code is ugly and the behaviour is going to be more surprising than before since now the document can be running normally except that its images (perhaps only some of its images) are not animated.
Assignee | ||
Comment 41•15 years ago
|
||
Well even now if you have same page loaded or same images in two windows, the animations in both windows are stopped.
Assignee | ||
Comment 42•15 years ago
|
||
...but sure, cloning images for printing would be the better.
Assignee | ||
Comment 43•15 years ago
|
||
I was hoping that keeping the current behavior with plugins and images would be enough for now.
Plugins yes, because we're not adding any significant new code for them. But for images you're adding these global arrays and mPrintImages...
Assignee | ||
Comment 45•15 years ago
|
||
(In reply to comment #44) > But > for images you're adding these global arrays and mPrintImages... because I want to fix the current problems we have. We may start animating images in print preview sort of randomly, although painting them depends on whether they are invalidated.
Assignee | ||
Comment 46•15 years ago
|
||
The patch does trigger those problems more often than what happens currently, but the problems are there even now.
If we snapshotted images for !IsDynamic presentations, wouldn't that fix the current problems too?
Assignee | ||
Comment 48•15 years ago
|
||
(In reply to comment #47) > If we snapshotted images for !IsDynamic presentations, wouldn't that fix the > current problems too? Yeah, that is what I'd like to do, sure, but fixing the current animation handling was just significantly easier, at least for me. No need to change layout/style or anything in like that. Certainly something to fix, but perhaps in a followup. Though, if wanted, I could hack that change to this bug too.
Assignee | ||
Comment 49•15 years ago
|
||
I'll try something a bit different what I thought earlier. I may need to push some bit up to imgloader.
Assignee | ||
Comment 50•15 years ago
|
||
(In reply to comment #49) > I'll try something a bit different what I thought earlier. > I may need to push some bit up to imgloader. Ok, the idea I had wouldn't work. Need something else. Have to hack stylesheet cloning to get that right.
Assignee | ||
Comment 51•15 years ago
|
||
(In reply to comment #37) > Why can't you pass 'aCloningForPrint' down through nsNodeUtils::Clone and > nsNodeUtils::CloneAndAdopt? That doesn't quite work. nsNodeUtils::CloneAndAdopt relies on DOM's Clone method and I can't add new parameters to that. So I think the current setup is (unfortunately) the simplest way to handle this.
(In reply to comment #51) > That doesn't quite work. nsNodeUtils::CloneAndAdopt relies on DOM's Clone > method > and I can't add new parameters to that. So I think the current setup is > (unfortunately) the simplest way to handle this. Oh well... > (In reply to comment #47) > Certainly something to fix, but perhaps in a followup. > Though, if wanted, I could hack that change to this bug too. I just don't like adding new code and later taking it out again. How about in this patch we don't try to handle image animation at all, then you do another patch on top of this patch to snapshot images, and then we land them together?
Assignee | ||
Comment 53•15 years ago
|
||
(In reply to comment #52) > How about in this patch we don't try to handle image animation at all, then you > do another patch on top of this patch to snapshot images, and then we land them > together? Well, I don't want to land something which breaks things badly (as I said the image animation problem is there already now, but the patch makes it even more visible). So I guess I need to change stylesheet cloning or something to take snapshots. And do that in this bug.
OK, but it can still be a separate patch, which would be helpful. One thing that might be a good idea is have the code use the term "CloneStatic" or "CloneStaticSnapshot" instead of "CloneForPrint", since that's more precisely what we're doing and might be useful for other things.
Assignee | ||
Comment 55•15 years ago
|
||
(In reply to comment #54) > OK, but it can still be a separate patch, which would be helpful. So you want to remove the imageanimation handling in this bug or in a followup? > One thing that might be a good idea is have the code use the term "CloneStatic" > or "CloneStaticSnapshot" instead of "CloneForPrint", since that's more > precisely what we're doing and might be useful for other things. OK
Assignee | ||
Comment 56•15 years ago
|
||
Just small changes in this one.
Attachment #399229 -
Attachment is obsolete: true
Attachment #401078 -
Flags: review?(roc)
Attachment #399229 -
Flags: review?(roc)
(In reply to comment #55) > (In reply to comment #54) > > OK, but it can still be a separate patch, which would be helpful. > So you want to remove the imageanimation handling in this bug or in a followup? I think: take the image animation handling out of this patch. Implement image snapshotting in an additional patch, in this bug.
Assignee | ||
Comment 58•15 years ago
|
||
... so I guess I'll take out the image animation and update the patch to whatever the solution will be. stylesheet cloning isn't too helpful here unfortunately.
At style resolution time for the print document, when we create the imgRequest objects, can't we test if the document is static and if it is, and we already have an image frame (because we got a reference to an existing image), snapshot at that point?
Assignee | ||
Comment 60•15 years ago
|
||
Ah, perhaps at a bit later than what I thought. Stylesheet would still use the original image request.
Assignee | ||
Comment 61•15 years ago
|
||
But I still wonder who keeps the image request alive...
Assignee | ||
Comment 62•15 years ago
|
||
Or I guess, basically how to pass the context info down to nsStyleImage
Assignee | ||
Comment 63•15 years ago
|
||
Assignee | ||
Comment 64•15 years ago
|
||
This clones the current frame of animated images.
Attachment #401078 -
Attachment is obsolete: true
Attachment #401193 -
Flags: review?(roc)
Attachment #401078 -
Flags: review?(roc)
+ PRPackedBool mIsStaticDocument; + + PRPackedBool mCloningForStatic; + + nsCOMPtr<nsIDocument> mOriginalDocument; Add comments here + // Make document to use different container during cloning. "Make document use" + + } else if (mDecoder) mDecoder->Paint(aContext, aFilter, aRect); Lose blank line and add {} + if (frame && frame->GetType() == nsGkAtoms::HTMLVideoFrame && + static_cast<nsVideoFrame*>(frame)->ShouldDisplayPoster()) { + elem = do_QueryInterface(static_cast<nsVideoFrame*>(frame)-> + GetPosterImage()); + } else { + elem = do_QueryInterface(const_cast<nsHTMLMediaElement*>(this)); + } + + nsLayoutUtils::SurfaceFromElementResult res = + nsLayoutUtils::SurfaceFromElement(elem, + nsLayoutUtils::SFE_WANT_NEW_SURFACE); This happens to have the effect that in a print presentation, the poster image will be scaled to preserve aspect ratio. (Whereas currently the poster image is scaled to fill the frame.) But that's OK because we really want the former behaviour anyway, and we'll make that happen in bug 517363.
Blocks: 468568
Comment on attachment 401193 [details] [diff] [review] +animated image cloning + if (GetIsPrintPreview() || + (mDocument && mDocument->IsStaticDocument())) How can we be in print preview with a non-static document? +#define NS_SET_IMAGE_REQUEST(method_, context_, request_) \ + if (context_->PresContext()->IsDynamic()) { \ + method_(request_); \ + } else { \ + nsCOMPtr<imgIRequest> req = nsContentUtils::GetStaticRequest(request_); \ + method_(req); \ + } Why not just have a helper function imgIRequest* MakeStaticIfNeeded(nsStyleContext* aContext, imgIRequest* aRequest); ? dbaron should review the style system changes. Someone else will have to review the content changes too (jst?) and someone else again will have to review the imglib changes (joe?). I like this very much. Thank you. I wonder if we can simplify the printing of subdocuments in the future so we don't need a tree of nsPrintObjects. Something to think about. On trunk we have a bug where printing doesn't work properly with @font-face font loading --- bug 468568. This will help us fix that bug because we'll be able to reconstruct frames for a print document.
Attachment #401193 -
Flags: review?(roc) → review+
Assignee | ||
Comment 67•15 years ago
|
||
(In reply to comment #66) > Why not just have a helper function > imgIRequest* MakeStaticIfNeeded(nsStyleContext* aContext, imgIRequest* > aRequest); > ? Because using the macro I can easily have the nsCOMPtr to keep the already_addrefed object alive while setting the value. > (From update of attachment 401193 [details] [diff] [review]) > + if (GetIsPrintPreview() || > + (mDocument && mDocument->IsStaticDocument())) > > How can we be in print preview with a non-static document? I'll check this, but IIRC this might happen just *before* we enter print preview.
Assignee | ||
Comment 68•15 years ago
|
||
Attachment #401420 -
Flags: review?(jst)
Assignee | ||
Updated•15 years ago
|
Attachment #401420 -
Flags: review?(dbaron)
Assignee | ||
Updated•15 years ago
|
Attachment #401420 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 69•15 years ago
|
||
Comment on attachment 401420 [details] [diff] [review] v20.1 jst, could you look at the content changes? dbaron, could you check the style handling changes? Bholley, this has the dummy request we talked about on IRC.
Assignee | ||
Comment 70•15 years ago
|
||
Seems like I need to change + img->GetAnimated(&animated); + if (!animated) { + NS_ADDREF(*aReturn = aRequest); + return NS_OK; + } to nsresult rv = nsimg->GetAnimated(&animated); if (NS_SUCCEED(rv) && !animated) { NS_ADDREF(*aReturn = aRequest); return NS_OK; } once bug 517543 is fixed.
Depends on: 517543
Comment 71•15 years ago
|
||
I'm liking the imgDummyRequest name less and less, but I don't really have any other good ideas. Joe - thoughts? + nsCOMPtr<imgIContainer> img, currentFrame; + aRequest->GetImage(getter_AddRefs(img)); + if (img) { + PRBool animated = PR_FALSE; + img->GetAnimated(&animated); + if (!animated) { + NS_ADDREF(*aReturn = aRequest); + return NS_OK; + } Per our discussion on IRC, you can't rely on this as is without bug 517543. + // Lock dummy requests for now, since they can't trigger onDiscard. + req->LockImage(); This isn't necessary assuming the conditional approach. Assuming that the image was indeed animated, extracting the frame definitely creates a new imgContainer, and that imgContainer has no source data (so it can't discard). + nsCOMPtr<imgIDecoderObserver> kungFuDeathGrip2(aObserver); why "2"? + aObserver->OnStartRequest(req); OnStartRequest should fire unconditionally. + if (req->mImageStatus & STATUS_LOAD_COMPLETE) { + aObserver->OnStartDecode(req); + aObserver->OnStartFrame(req, 0); + nsIntRect r; + mImage->GetCurrentFrameRect(r); + aObserver->OnDataAvailable(req, 0, &r); + aObserver->OnStopFrame(req, 0); + } + if (req->mImageStatus & STATUS_FRAME_COMPLETE) { + aObserver->OnStopContainer(req, req->mImage); + aObserver->OnStopDecode(req, NS_OK, nsnull); + aObserver->OnStopRequest(req, PR_TRUE); + } These don't look right to me. See the IDL docs in imgIDecoderObserver for the distinction between load notifications and decode notifications (note that, at present, OnStopDecode should be treated as a load notification and should be fired right before OnStopRequest). OnStartFrame, OnDataAvailable, and OnStopFrame should be conditional on STATUS_FRAME_COMPLETE, and OnStopContainer (pooor man's OnStopDecode for the moment) should be contingent on STATUS_DECODE_COMPLETE (introduced in bug 517543). +NS_IMETHODIMP +imgDummyRequest::RequestDecode() +{ + return mImage ? mImage->RequestDecode() : NS_OK; +} +NS_IMETHODIMP +imgDummyRequest::LockImage() +{ + if (mImage) { + ++mLocksHeld; + return mImage->LockImage(); + } + return NS_OK; +} + +NS_IMETHODIMP +imgDummyRequest::UnlockImage() +{ + if (mImage) { + NS_ABORT_IF_FALSE(mLocksHeld > 0, "calling unlock but no locks!"); + --mLocksHeld; + return mImage->UnlockImage(); + } + return NS_OK; +} + +imgDummyRequest::~imgDummyRequest() +{ + if (mImage) { + while (mLocksHeld) { + UnlockImage(); + } + } +} I don't like the mImage checks here. Can't we just mandate that mImage is non-null and assert it in the constructor?
Updated•15 years ago
|
Attachment #401420 -
Flags: review?(bobbyholley) → review-
Assignee | ||
Comment 72•15 years ago
|
||
(In reply to comment #71) > + nsCOMPtr<imgIDecoderObserver> kungFuDeathGrip2(aObserver); > why "2"? Leftover > > + aObserver->OnStartRequest(req); > > OnStartRequest should fire unconditionally. ok. > > + if (req->mImageStatus & STATUS_LOAD_COMPLETE) { > + aObserver->OnStartDecode(req); > + aObserver->OnStartFrame(req, 0); > + nsIntRect r; > + mImage->GetCurrentFrameRect(r); > + aObserver->OnDataAvailable(req, 0, &r); > + aObserver->OnStopFrame(req, 0); > + } > + if (req->mImageStatus & STATUS_FRAME_COMPLETE) { > + aObserver->OnStopContainer(req, req->mImage); > + aObserver->OnStopDecode(req, NS_OK, nsnull); > + aObserver->OnStopRequest(req, PR_TRUE); > + } > > These don't look right to me. See the IDL docs in imgIDecoderObserver for the > distinction between load notifications and decode notifications (note that, at > present, OnStopDecode should be treated as a load notification and should be > fired right before OnStopRequest). imgIDecoderObserver doesn't explain these too well. I was *trying* to do whatever imgRequest does :) > (pooor man's OnStopDecode for the moment) should be contingent on > STATUS_DECODE_COMPLETE (introduced in bug 517543). OK > I don't like the mImage checks here. Why not? > Can't we just mandate that mImage is > non-null and assert it in the constructor? How can I mandate that. I need a request which will never point to an animated image. If a request is cloned to a dummy request before imgRequest has mImage, I still need something to return. And that something shouldn't be imgRequestProxy, since imgRequest may get the mImage once OnDataAvailable is called. So I'd rather return a dummy request without mImage.
Comment 73•15 years ago
|
||
(In reply to comment #72) > > I don't like the mImage checks here. > Why not? > > > Can't we just mandate that mImage is > > non-null and assert it in the constructor? > How can I mandate that. I need a request which will never point to an > animated image. If a request is cloned to a dummy request before > imgRequest has mImage, I still need something to return. And that something > shouldn't be imgRequestProxy, since imgRequest may get the mImage once > OnDataAvailable is called. So I'd rather return a dummy request without mImage. My general gripe here (and following) is that imgIRequests are supposed to be dynamic. They may not have everything _now_, but they'll probably get it sometime soon. imgIRequest guarantees, for example, that locks applied now will be deferred and applied later if the image doesn't exist yet. But in this case, the image is never going to exist. Nonetheless, I don't really see a way around it. To mitigate the issues, and make things as close to consistent as possible, you assert in the constructor that _either_ mImage is null _or_ the number of frames is exactly equal to 1, and then set up state and notifications this way: OnStartRequest if (!mImage) { OnStopDecode(error); OnStopRequest(); mImageStatus = STATUS_ERROR; return; } mImageStatus = 0; if (SIZE_AVAILABLE) { mImageStatus |= STATUS_SIZE_AVAILABLE; OnStartContainer() } OnStartDecode() OnStartFrame() OnDataAvailable() mImageStatus |= STATUS_FRAME_COMPLETE OnStopFrame() mImage |= STATUS_DECODE_COMPLETE OnStopContainer() mImage |= STATUS_LOAD_COMPLETE OnStopDecode() OnStopRequest() And of course, joe's going to need to take a look at this as well.
Assignee | ||
Comment 74•15 years ago
|
||
(In reply to comment #73) > My general gripe here (and following) is that imgIRequests are supposed to be > dynamic. But what I need is static request :) > imgIRequest guarantees, for example, that locks applied now will > be deferred and applied later if the image doesn't exist yet. But in this case, > the image is never going to exist. Well, nothing guarantees that normal imgRequest gets the mImage either. > assert in the constructor > that _either_ mImage is null _or_ the number of frames is exactly equal to 1 Makes sense. > and then set up state and notifications this way: You mean in Clone()? Why do I need to change image status between OnXXX calls? That is not what happens when normal imgIRequest is cloned.
Comment 75•15 years ago
|
||
(In reply to comment #74) > (In reply to comment #73) > You mean in Clone()? > Why do I need to change image status between OnXXX calls? > That is not what happens when normal imgIRequest is cloned. You don't really. I just wrote it that way because the notifications sent from Clone() are supposed to be a simulation of all the steps that happened on the request before the clone occurred. Feel free to set them all at the beginning, so long as the effect is the same.
Comment 76•15 years ago
|
||
Comment on attachment 401420 [details] [diff] [review] v20.1 + virtual already_AddRefed<nsIDocument> + CloneForStatic(nsISupports* aCloneContainer); I'd suggest we name these Clone*ForStatic() methods something like CreateStatic*Clone instead, throughout the patch. r=jst with that and others' comments addressed.
Attachment #401420 -
Flags: review?(jst) → review+
Attachment #401420 -
Flags: review?(dbaron) → review+
Comment on attachment 401420 [details] [diff] [review] v20.1 nsRuleNode.cpp: +#define NS_SET_IMAGE_REQUEST(method_, context_, request_) \ + if (context_->PresContext()->IsDynamic()) { \ Parenthesize: (context_) rather than context_ nsStyleStruct.h: >- >+private: > nsCOMPtr<imgIRequest> mBorderImage; // [reset] > >+protected: Just make the whole section private; nothing derives from this. >+ void SetListStyleImage(imgIRequest* aReq); Can the setter be inline, or does that introduce header dependency problems? + void SetImage(imgIRequest* aRequest); Same here. r=dbaron on the layout/style changes with that fixed. Sorry for the delay.
Assignee | ||
Comment 78•15 years ago
|
||
(In reply to comment #77) > >+ void SetListStyleImage(imgIRequest* aReq); > > Can the setter be inline, or does that introduce header dependency problems? IIRC, header dependency problems were the reason for not inlining, but I'll retest.
Assignee | ||
Comment 79•15 years ago
|
||
Joe, could you review the libpr0n/ parts. I changed imgDummyRequest::Clone based on comment 73.
Attachment #401420 -
Attachment is obsolete: true
Attachment #412265 -
Flags: review?(joe)
Assignee | ||
Comment 80•15 years ago
|
||
1) DummyRequest->ContainerRequest 2) imgITools::getStaticRequest->imgIRequest::getStaticRequest 3) Use similar code for imgContainerRequest::Clone as what imgRequest::NotifyProxyListener has. I'm not too happy with copying that code, but it seemed like the simplest way to keep notification handling similar in both cases. And fortunately it isn't too many lines of code.
Attachment #412265 -
Attachment is obsolete: true
Attachment #412302 -
Flags: review?(joe)
Attachment #412265 -
Flags: review?(joe)
Assignee | ||
Comment 81•15 years ago
|
||
There was canvas painting missing. But still, joe, please review the imglib part.
Attachment #412302 -
Attachment is obsolete: true
Attachment #412328 -
Flags: review?(joe)
Attachment #412302 -
Flags: review?(joe)
Assignee | ||
Comment 82•15 years ago
|
||
This has simpler canvas handling, but webgl handling is broken because of Bug 528746.
Attachment #412328 -
Attachment is obsolete: true
Attachment #412430 -
Flags: review?(joe)
Attachment #412328 -
Flags: review?(joe)
Assignee | ||
Comment 83•15 years ago
|
||
Comment on attachment 412430 [details] [diff] [review] v24 Roc, does the <canvas> handling look ok to you? Look for nsHTMLCanvasElement::CopyInnerTo
Attachment #412430 -
Flags: review?(roc)
Comment on attachment 412430 [details] [diff] [review] v24 Yes, that looks fine. What are we going to do for tests here?
Attachment #412430 -
Flags: review?(roc) → review+
It would be nice to have some tests that make static clones of documents with different kinds of content and then use WindowSnapshot to make sure they look the same. And we should also be able to use WindowSnapshot to take a snapshot of the original documents, mutate them, and then take a snapshot of the static document to verify that it really is static.
Assignee | ||
Comment 86•15 years ago
|
||
Adds a test for 2d canvas print preview.
Attachment #412430 -
Attachment is obsolete: true
Attachment #412829 -
Flags: review?(joe)
Attachment #412430 -
Flags: review?(joe)
Comment 87•15 years ago
|
||
Comment on attachment 412829 [details] [diff] [review] v24.1 >diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h >+class imgITools; >+ static imgITools* sImgTools; >diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp >+#include "imgITools.h" >+imgITools* nsContentUtils::sImgTools = nsnull; >+ rv = CallCreateInstance("@mozilla.org/image/tools;1", &sImgTools); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ NS_IF_RELEASE(sImgTools); should all be removed since we're not using imgITools any more. >diff --git a/modules/libpr0n/src/Makefile.in b/modules/libpr0n/src/Makefile.in > CPPSRCS = \ > imgContainer.cpp \ > imgFrame.cpp \ > imgLoader.cpp \ > imgRequest.cpp \ > imgRequestProxy.cpp \ >- imgTools.cpp >+ imgTools.cpp \ >+ imgContainerRequest.cpp While you're in here, will you change this to add a $(NULL) so this makefile's like every other Mozilla makefile? >diff --git a/modules/libpr0n/src/imgRequestProxy.cpp b/modules/libpr0n/src/imgRequestProxy.cpp >+NS_IMETHODIMP >+imgRequestProxy::GetStaticRequest(imgIRequest** aReturn) >+{ >+ *aReturn = nsnull; >+ nsCOMPtr<imgIContainer> img, currentFrame; >+ GetImage(getter_AddRefs(img)); >+ if (img) { >+ PRBool animated = PR_FALSE; >+ nsresult rv = img->GetAnimated(&animated); >+ if (NS_SUCCEEDED(rv) && !animated) { >+ NS_ADDREF(*aReturn = this); >+ return NS_OK; >+ } >+ >+ PRInt32 w = 0; >+ PRInt32 h = 0; >+ img->GetWidth(&w); >+ img->GetHeight(&h); >+ nsIntRect rect(0, 0, w, h); >+ img->ExtractFrame(imgIContainer::FRAME_CURRENT, rect, >+ imgIContainer::FLAG_SYNC_DECODE, >+ getter_AddRefs(currentFrame)); >+ } >+ >+ nsCOMPtr<nsIURI> uri; >+ GetURI(getter_AddRefs(uri)); >+ PRUint32 imageStatus = 0; >+ GetImageStatus(&imageStatus); >+ nsCOMPtr<nsIPrincipal> principal; >+ GetImagePrincipal(getter_AddRefs(principal)); >+ >+ imgContainerRequest* req = >+ new imgContainerRequest(currentFrame, uri, imageStatus, >+ mOwner ? mOwner->GetState() : 0, >+ principal); >+ NS_ENSURE_TRUE(req, NS_ERROR_OUT_OF_MEMORY); Shouldn't use NS_ENSURE_TRUE - it's not worthy of an NS_WARNING that we're out of memory, we should just handle it. >+ NS_ADDREF(*aReturn = req); >+ return NS_OK; >+} It isn't valid for an imgContainerRequest to be created with a null currentFrame (at least in Clone() we unconditionally dereference its frame), so I'd rather we early-exit if we have no image. >+ >diff --git a/modules/libpr0n/src/imgTools.cpp b/modules/libpr0n/src/imgTools.cpp >--- a/modules/libpr0n/src/imgTools.cpp >+++ b/modules/libpr0n/src/imgTools.cpp >@@ -215,8 +215,9 @@ NS_IMETHODIMP imgTools::EncodeScaledImag > rv = encoder->InitFromData(bitmapData, bitmapDataLength, > aScaledWidth, aScaledHeight, strideSize, > imgIEncoder::INPUT_FORMAT_HOSTARGB, EmptyString()); > > NS_ENSURE_SUCCESS(rv, rv); > > return CallQueryInterface(encoder, aStream); > } >+ Left-over, unrelated change. >+imgContainerRequest::imgContainerRequest(imgIContainer* aImage, >+#ifdef DEBUG >+ PRUint32 numFrames = 0; >+ if (aImage) { >+ aImage->GetNumFrames(&numFrames); >+ } >+ NS_ASSERTION(!aImage || numFrames == 1, >+ "Shouldn't have image with more than one frame!"); NS_ABORT_IF_FALSE >+NS_IMETHODIMP >+imgContainerRequest::Clone(imgIDecoderObserver* aObserver, imgIRequest** _retval) >+{ >+ imgContainerRequest* req = >+ new imgContainerRequest(mImage, mURI, mImageStatus, mState, mPrincipal); >+ NS_ENSURE_TRUE(req, NS_ERROR_OUT_OF_MEMORY); Don't use NS_ENSURE_TRUE for OOM.
Attachment #412829 -
Flags: review?(joe) → review-
Assignee | ||
Comment 88•15 years ago
|
||
Comments addressed except "I'd rather we early-exit if we have no image" I really want to get a dummy request whenever possible. That way I can use the dummy request and not the real one, but yet have something else than just null. And imgContainerRequest::Clone does have null checks.
Attachment #401193 -
Attachment is obsolete: true
Attachment #412829 -
Attachment is obsolete: true
Attachment #412946 -
Flags: review?(joe)
Updated•15 years ago
|
Attachment #412946 -
Flags: review?(joe) → review+
Assignee | ||
Comment 89•15 years ago
|
||
I pushed this patch.
Comment 91•15 years ago
|
||
Should this be marked FIXED?
Assignee | ||
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 92•14 years ago
|
||
Bug 535343
Comment 93•14 years ago
|
||
Comment on attachment 417119 [details] [diff] [review] v29 >+ this._printPreviewTab = tabbrowser.loadOneTab("about:blank", null, null, >+ null, true, false); Sadly most print preview browsers do not implement loadOneTab. These include * View source * SeaMonkey * Thunderbird Can you easily remove the dependency or do you need to back this out?
Assignee | ||
Comment 94•14 years ago
|
||
This won't be backed out, for sure. I'll fix the toolkit/ mess. Sorry about that. Fortunately the code changes there are quite small.
Comment 95•14 years ago
|
||
Comment on attachment 417119 [details] [diff] [review] v29 >+ this._printPreviewTab = tabbrowser.loadOneTab("about:blank", null, null, >+ null, true, false); In fact, why bother with loadOneTab(...) when addTab() will do?
Comment 96•14 years ago
|
||
> In fact, why bother with loadOneTab(...) when addTab() will do?
Thunderbird's addTab() implementation is different. I think you need to specify a contentTab as a parameter.
Assignee | ||
Comment 97•14 years ago
|
||
Please don't add comments here, but to the relevant bugs. Thanks.
Comment 98•12 years ago
|
||
Hello Olli, Do you know how to printPreview a page in an embedded xulrunner application with your this patch? Or there's some guide for it? I can print preview a page in my xulrunner embed app with below snippet code before(xulrunner1.9.2): nsIWebBrowserPrint::PrintPreview(globalPrintSettings, null, null); But after apply your patch(xulrunner2.0), it not work, since the second parameter of nsIWebBrowserPrint::PrintPreview() should not be null which ruled in this patch. I'm tried to pass the current browser window to it, but not work, an blank page displayed. Could you help me on it? Thank you.
Assignee | ||
Comment 99•12 years ago
|
||
You could open a new "tab" or "window" (whatever your xulrunner app has) or just any xul:browser or html:iframe works too and do something similar to http://mxr.mozilla.org/mozilla-central/source/layout/base/tests/chrome/printpreview_helper.xul#26 or http://mxr.mozilla.org/mozilla-central/source/toolkit/components/printing/content/printUtils.js Basically you open a new browsing context and clone/printpreview the original document there.
Comment 100•12 years ago
|
||
Many thanks for your comments. I created a new browser for print preview, get the nsIWebBrowserPrint instance from the new created browser, and pass the original document window to PrintPreview() method, but there still display a blank window. Need I manually clone the original content to print preview window? Below is my code, is there some wrong in it? I read the code in printpreview_helper.xul carefully, seems my code flow same as it. Not sure why it not work. I debugged, each sentence code executed succeed, not failure. Could you help give some comments on it? Shell shell = new Shell(); shell.setLayout(new FillLayout()); // create a new browser for print preview DOMBrowser browser = new DOMBrowser(shell, SWT.NULL); shell.open(); int[] address = new int[1]; nsIWebBrowser newWb = browser.getWebBrowser(); int rc = newWb.QueryInterface(nsIInterfaceRequestor.NS_IINTERFACEREQUESTOR_IID, address); nsIInterfaceRequestor req = new nsIInterfaceRequestor(address[0]); rc = req.GetInterface(nsIDocShell.NS_IDOCSHELL_IID,address); req.Release(); nsIDocShell docShell = new nsIDocShell(address[0]); docShell.GetPrintPreview(address); // get the nsIWebBrowserPrint from new created browser printMoz = new nsIWebBrowserPrint(address[0]); nsIWebBrowser wb = this.getWebBrowser(); int[] address2=new int[1]; rc = wb.GetContentDOMWindow(address); // get current document's nsIDOMWindow int[] printSettings = new int[1]; printMoz.GetGlobalPrintSettings(printSettings); rc = printMoz.PrintPreview(printSettings[0], address2[0], 0); // printpreview it
Assignee | ||
Comment 101•12 years ago
|
||
(In reply to Yun Guo from comment #100) > Many thanks for your comments. I created a new browser for print preview, > get the nsIWebBrowserPrint instance from the new created browser, and pass > the original document window to PrintPreview() method, but there still > display a blank window. Need I manually clone the original content to print > preview window? No > nsIDocShell docShell = new nsIDocShell(address[0]); What does this do? Is the docshell attached to somewhere?
Comment 102•12 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #101) > > nsIDocShell docShell = new nsIDocShell(address[0]); > What does this do? Is the docshell attached to somewhere? This line code is to get the new created browser's nsIDocShell, I want to from which to get the nsIWebBrowserPrint interface. No, this docshell is not attached to other place. Today, I tried below code which is query the nsIWebBrowserPrint interface directly from new browser, but still can't display the content in print preview window, still a blank window. Seems the printPreview method not work in embed mode or something wrong in my code? Shell shell = new Shell(); shell.setLayout(new FillLayout()); // create a new browser for print preview DOMBrowser browser = new DOMBrowser(shell, SWT.NULL); shell.open(); int[] address = new int[1]; nsIWebBrowser newWb = browser.getWebBrowser(); int rc = newWb.QueryInterface(nsIInterfaceRequestor.NS_IINTERFACEREQUESTOR_IID, address); nsIInterfaceRequestor req = new nsIInterfaceRequestor(address[0]); // get the nsIWebBrowserPrint from new created browser rc = req.GetInterface(nsIWebBrowserPrint.NS_IWEBBROWSERPRINT_IID,address); req.Release(); printMoz = new nsIWebBrowserPrint(address[0]); nsIWebBrowser wb = this.getWebBrowser(); int[] address2=new int[1]; rc = wb.GetContentDOMWindow(address); // get current document's nsIDOMWindow int[] printSettings = new int[1]; printMoz.GetGlobalPrintSettings(printSettings); rc = printMoz.PrintPreview(printSettings[0], address2[0], 0); // printpreview it
Assignee | ||
Comment 103•12 years ago
|
||
(In reply to Yun Guo from comment #102) > Shell shell = new Shell(); > shell.setLayout(new FillLayout()); > // create a new browser for print preview > DOMBrowser browser = new DOMBrowser(shell, SWT.NULL); > shell.open(); > > int[] address = new int[1]; > nsIWebBrowser newWb = browser.getWebBrowser(); > > int rc = > newWb.QueryInterface(nsIInterfaceRequestor.NS_IINTERFACEREQUESTOR_IID, > address); > nsIInterfaceRequestor req = new nsIInterfaceRequestor(address[0]); > > // get the nsIWebBrowserPrint from new created browser > rc = req.GetInterface(nsIWebBrowserPrint.NS_IWEBBROWSERPRINT_IID,address); > req.Release(); > printMoz = new nsIWebBrowserPrint(address[0]); > > nsIWebBrowser wb = this.getWebBrowser(); > int[] address2=new int[1]; > rc = wb.GetContentDOMWindow(address); // get current document's nsIDOMWindow > > int[] printSettings = new int[1]; > printMoz.GetGlobalPrintSettings(printSettings); > rc = printMoz.PrintPreview(printSettings[0], address2[0], 0); Why you pass 'address2'? it doesn't seem to point to anywhere. Perhaps you mean to store the address of current window in it, but you're actually using 'address' We should move this discussion to email.
Comment 104•12 years ago
|
||
Sorry, there's a typo mistake in line: "rc = wb.GetContentDOMWindow(address);" when I write that comments. But it's correct in my project.
Comment 105•12 years ago
|
||
rc = wb.GetContentDOMWindow(address); should be rc = wb.GetContentDOMWindow(address2);
Comment 106•12 years ago
|
||
I write a simple html test case and verified that the printPreview is not work to print preview into a new window in embed mode. Within the html file, the printPreviewInNewWindow() js method is to print preview the frame content in the new opened window. I tested, it worked in firefox, but not work in our embed xulrunner. The printpreview window is empty in embed mode, same result as execution of above snippet code. I attached the html file here. <html> <body> <iframe src="http://www.baidu.com" type="content"></iframe> <input type="button" value="openNewWindow" onclick="createNewWindow()"> <input type="button" value="printPreview in NewWindow" onclick="printPreviewInNewWindow()"> <script> var gWbp; var newWindow; function createNewWindow() { newWindow=window.open("about:blank"); } function printPreviewInNewWindow() { netscape.security.PrivilegeManager.enablePrivilege( "UniversalXPConnect"); gWbp = newWindow.QueryInterface (Components.interfaces.nsIInterfaceRequestor) .getInterface(Components.interfaces.nsIWebBrowserPrint); gWbp.printPreview(gWbp.globalPrintSettings, window.frames[0], null); } </script> </body> </html>
Comment 107•12 years ago
|
||
Olli, do you know is there some other special things to do for embed xulrunner to printpreview a page in new window?
Assignee | ||
Comment 108•12 years ago
|
||
You do get any useful warnings to the terminal (I assume you're using a debug build). And please send me email. This discussion shouldn't happen in this bug.
Updated•5 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•