Last Comment Bug 115136 - [FIX]Enable developers to fly their print dialog
: [FIX]Enable developers to fly their print dialog
Status: VERIFIED FIXED
: topembed+
Product: Core
Classification: Components
Component: Printing: Output (show other bugs)
: Trunk
: x86 Windows 2000
: P1 normal (vote)
: mozilla1.0
Assigned To: rods (gone)
: sujay
: Jet Villegas (:jet)
Mentors:
Depends on:
Blocks: 125824 98995 99619 108502 124777 130094 141849 143241 362840 362841
  Show dependency treegraph
 
Reported: 2001-12-13 15:28 PST by rods (gone)
Modified: 2002-08-23 11:24 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Phase #2 patch (236.89 KB, patch)
2002-04-26 08:49 PDT, rods (gone)
no flags Details | Diff | Splinter Review
patch (264.85 KB, patch)
2002-05-02 13:26 PDT, rods (gone)
dcone: review+
attinasi: superreview+
Details | Diff | Splinter Review
Documentation (7.83 KB, text/html)
2002-05-02 16:43 PDT, rods (gone)
no flags Details
this is what was checked in (391.24 KB, patch)
2002-05-16 06:08 PDT, rods (gone)
jud: approval+
Details | Diff | Splinter Review
patch against the branch (133.79 KB, patch)
2002-05-17 13:22 PDT, rods (gone)
no flags Details | Diff | Splinter Review
Merged against the Branch (437.01 KB, patch)
2002-05-17 13:25 PDT, rods (gone)
jud: approval+
Details | Diff | Splinter Review

Description rods (gone) 2001-12-13 15:28:11 PST
 
Comment 1 Simon Fraser 2002-01-30 12:37:18 PST
Can you explain this bug a little better?
Comment 2 rods (gone) 2002-02-01 10:13:44 PST
The solution for this bug will be to introduce a "pluggable" dialog service.
Just like the network libs use the nsIPrompt mechanism for flynig dialogs and
embedders can override them with their own dialogs.

My guess is that much like what we do for netwerk, we will pass in a
nsIInterfaceRequestor for callbacks and then do a GetInterface on the callbacks
for the nsIPrintingPrompt interface.

The implementation of that can then be elsewhere like in the embedding
directory. The a particular platform can choose to use the nsIInterfaceRequestor
or just fly their own native one.

On Windows we may move entirely away from the XPDialog, but certainly for Linux
we need to do this.
Comment 3 Simon Fraser 2002-02-01 11:11:17 PST
I assume this solution would remove entirely the dependencies in gfx on dom and 
windowwatcher? Would gfx become dependent on embedcomponents, or would print 
dialogs be invoked entirely from outside of gfx?
Comment 4 Kevin McCluskey (gone) 2002-02-07 14:55:58 PST
Marking nsbeta1+.
Comment 5 Syd Logan 2002-03-21 22:31:49 PST
nsbeta1- per adt triage
Comment 6 Syd Logan 2002-03-21 22:37:42 PST
topembed+ per adt triage
Comment 7 Roland Mainz 2002-04-08 07:38:22 PDT
rods:
How does this affect bug 136058 ("Implement nsPrinterFeatures" - the idea of
this RFE is to create a service to query printer properties/attributes like
available paper sizes/orientations for a specified printer...) ?
AFAIK embedders would need this info to "fly their" print dialog without using
hardcoded values in their print dialog (currently both PostScript module and
Xprint module use the printerfeatures prototype in
nsDeviceContextSpecGTK/nsDeviceContextSpecXlib - we only need to make the code a
XPCOM service... :)
Comment 8 rods (gone) 2002-04-26 08:49:32 PDT
Created attachment 81155 [details] [diff] [review]
Phase #2 patch

Phase #1 patch is in Bug 135441 I will attach an overview document soon.
Comment 9 rods (gone) 2002-05-02 13:26:50 PDT
Created attachment 82090 [details] [diff] [review]
patch

this is more up-to-date
Comment 10 Roland Mainz 2002-05-02 14:31:07 PDT
rods:
Since you are already touching all platforms - is it possible to implement the
API calls for bug 133258 ("Enhance nsIDeviceContext print API with an additional
level to support multiple documents in one print job") with this patch - or (at
least) rename s/BeginDocument/BeginJob/ and s/EndDocument/EndJob/, please ?
Comment 11 rods (gone) 2002-05-02 16:43:58 PDT
Created attachment 82120 [details]
Documentation

Read about Phase #2
Comment 12 Philip Langdale 2002-05-02 16:45:15 PDT
Glad to see that nsIPrintPromptService has landed, but, needless to say, a
pluggable print dialog system is useless if mozilla doesn't invoke print dialogs
through it. I hope rod's patch can land asap; we're (galeon) still getting
tortured by javascript.print()...
Comment 13 Philip Langdale 2002-05-02 16:47:33 PDT
Heh, spoke too soon eh? Roadmap looks good.
Comment 14 dcone (gone) 2002-05-03 08:39:53 PDT
Comment on attachment 82090 [details] [diff] [review]
patch

r=dcone
Comment 15 Marc Attinasi 2002-05-06 10:43:07 PDT
Comment on attachment 82090 [details] [diff] [review]
patch

sr=attinasi, based partially on email comments that testing has been thorough
and on all platforms
Comment 16 rods (gone) 2002-05-07 12:44:47 PDT
fixed
Comment 17 rods (gone) 2002-05-14 09:36:49 PDT
verifying, this has been working for a couple of weeks
Comment 18 scottputterman 2002-05-14 11:53:12 PDT
Let's get this verified by QA either on the trunk or if they are substantial,
please create a test build with the batch and the branch and let QA test that. 
Comment 19 sujay 2002-05-14 12:55:29 PDT
verified...
Comment 20 Dawn Endico 2002-05-15 21:48:48 PDT
this patch has a bunch of conflicts.
Comment 21 rods (gone) 2002-05-16 06:08:13 PDT
Created attachment 83866 [details] [diff] [review]
this is what was checked in

Here the patch of what was checked in and this is a list of things that had
changed:
1) Change to encode '\n' as '_' in printer names for prefs (apparently I was
testing this idea; took out the calling code but left this in, the code worked
fin ein testing)
2) The nsDeviceContextSpecOS2.cpp which had conflicts was changed to the one
mkapply sent me.
3) At the last minute we discovered that the attr isPrintPreview in the
nsIWebBrowserPrint was no longer being used, so I removed the two calls to it
in DocViewer, It was removed later from the IDL.
4) Several Mac makefile file changes that came out of the last round of builds
Conrad did
5) For reason I can't explain I must changed to debug output statments from
inside a "if (doDebug)" if statment in printjoboptions.js
6) A bunch of stuff removed from
embedding/browser/powerplant/source/CPrintAttachment.cpp and few lines added to
better use the new APIs (I reviewed) 
7) A change in an embedding make to use a renamed directory from gtk to
unixshared.
8) A bunch of changes to get the viewer app working with printing again.

So here is my justification for checking this all in without further reviews:
Conrad had just spent a lot of time diligently doing builds on Mac for Mac9,
Carbon, and OSX. Waiting any length of time might for re-reviews for I think
are fairly minor issues might require doing builds on the Mac again.

Item #1 - that is the only item of concern, but it works fine.
Item #3 - I didn't feel removing code was worth a re-review. 
Item #6 - I reviewed.
Item #8 - nobody but the layout-group cares about viewer and I am usually the
only one you uses the Print" menu item, so I figure that didn't really need to
be reviewed at this point, I didn't even have a bug on this, so obivously
nobody uses it.

Most of the others were makefile changes that cam out of Conrad's builds.
Comment 22 rods (gone) 2002-05-17 13:22:43 PDT
Created attachment 84098 [details] [diff] [review]
patch against the branch

This is the the patch against the branch, al the new embedding files are
already in on the branch, but not in the build.
Comment 23 rods (gone) 2002-05-17 13:25:57 PDT
Created attachment 84100 [details] [diff] [review]
Merged against the Branch

Attachment 84098 [details] [diff] is in Bug 135441

This attachment is the correct one
Comment 24 scottputterman 2002-05-20 17:16:46 PDT
adding adt1.0.0+.  Please get drivers approval and then check into the 1.0 branch.
Comment 25 scottputterman 2002-05-29 22:47:43 PDT
changing to adt1.0.1+ for checkin to the 1.0 branch.  Please get drivers
approval before checking in.
Comment 26 Judson Valeski 2002-06-04 16:12:57 PDT
please land on the 1.0.1 branch. once there, remove the "mozilla1.0.1+" keyword,
and add the "fixed1.0.1" keyword.
Comment 27 rods (gone) 2002-06-07 06:51:36 PDT
checked in on branch

Note You need to log in before you can comment on or make changes to this bug.