Closed Bug 520494 Opened 14 years ago Closed 14 years ago

Implement the Cocoa print dialog in a way that works on 10.4


(Core :: Widget: Cocoa, defect, P2)




Tracking Status
status1.9.2 --- beta2-fixed


(Reporter: mstange, Assigned: mstange)




(Whiteboard: [crashkill])


(5 files, 1 obsolete file)

It's funny how a quick look at an Appkit class dump can change things...

(In reply to bug 456646 comment #1)
> I'm using the following APIs that don't exist on 10.4:
>  1. -[NSPrintPanel showModalWithPrintInfo:]

As Tom Dyas predicted, this can be replaced with

NSPrintOperation* printOperation = [NSPrintOperation printOperationWithView:nil printInfo:printInfo];
[NSPrintOperation setCurrentOperation:printOperation];
[panel runModal];

>  2. -[NSPrintInfo PMPrintSettings/PMPrintSession/PMPageFormat]

These methods are present on 10.4 with slightly different names:
pmPrintSettings, _pmPrintSession and pmPageFormat (notice lower case pm and the underscore in front of pmPrintSession)

They are a private API.

>  3. -[NSString stringByReplacingOccurrencesOfString:withString:]

We can use -[NSMutableString replaceOccurrencesOfString:withString:options:range:].

>  4. -[NSPrintPanel addAccessoryController:]
>  5. PMPageFormatCreate(With)DataRepresentation

These are only necessary for 64 bit compatibility. Their 10.4 equivalents are setAccessorView and PM(Un)flattenPageFormat(To/With)CFData.


So this means that we can get rid of the configure stuff and throw away all of the old code.
I'm attaching an interdiff on top of the v1 patch in bug 456646 that makes it run on 10.4.
Consolidated complete patch coming later today.
Attached patch 10.4 changesSplinter Review
This is just a backout of bug 456646, provided for easier mq-ability.
Attached patch v1Splinter Review
New trunk patch with 10.4 support, applies on top of the backout.
Attachment #404550 - Flags: review?(joshmoz)
Attached patch v1, 1.9.2 branch version (obsolete) — Splinter Review
Like the trunk patch, just without #ifdefs and without any changes to *.properties files (because 1.9.2 is already string-frozen).
Blocks: 520394
Attachment #404550 - Flags: review?(joshmoz) → review+
Comment on attachment 404550 [details] [diff] [review]

+- (NSButton*)radioWithLabel:(const char*)aLabel andFrame:(NSRect)aRect;

This is not used.
Argh, I thought I had removed that again.
Philor mentioned that I probably want to remove the plugin from older builds when updating. Ted, do you know what exactly I have to add to This whole list? (That's the output of "find embedding/components/printingui/src/mac/printpde" in my objdir, minus the directories.)

> embedding/components/printingui/src/mac/printpde/Info-PrintPDE.plist
> embedding/components/printingui/src/mac/printpde/Makefile
> embedding/components/printingui/src/mac/printpde/PrintPDE.xcode/project.pbxproj
> embedding/components/printingui/src/mac/printpde/build/Development/PrintPDE.plugin/Contents/Info.plist
> embedding/components/printingui/src/mac/printpde/build/Development/PrintPDE.plugin/Contents/MacOS/PrintPDE
> embedding/components/printingui/src/mac/printpde/build/Development/PrintPDE.plugin/Contents/Resources/English.lproj
> embedding/components/printingui/src/mac/printpde/build/Development/PrintPDE.plugin/Contents/Resources/English.lproj/Localizable.strings
> embedding/components/printingui/src/mac/printpde/build/Development/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib
> embedding/components/printingui/src/mac/printpde/build/Development/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/classes.nib
> embedding/components/printingui/src/mac/printpde/build/Development/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/info.nib
> embedding/components/printingui/src/mac/printpde/build/Development/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/objects.xib
> embedding/components/printingui/src/mac/printpde/build/
> embedding/components/printingui/src/mac/printpde/build/
> embedding/components/printingui/src/mac/printpde/build/
> embedding/components/printingui/src/mac/printpde/build/
> embedding/components/printingui/src/mac/printpde/build/
> embedding/components/printingui/src/mac/printpde/build/
> embedding/components/printingui/src/mac/printpde/build/
Not that set, no: works against the contents of, not an objdir. Think of it as a script running from So according to |find ../Plug-Ins -type f| in the first random I found, it would be


But there are either two or three questions that come before "which files?"

* We've never actually used ../ in, although when I tested it a few months back it seemed to work. Do we really want to use it?

* What will happen when we have a Plug-Ins containing the empty directory structure that the updater will leave behind?

* Possibly part of the same question, what happens when we leave the contents of PrintPDE.plugin there, and is that better or worse than what happens with the empty directories?
FWIW, I don't really know how works at a functional level. I'd take philor's word for it. CCing Rob Strong for more insight.
Philor has it right. It is a list of files to remove if they exist on app update.
Attachment #404900 - Flags: review?(robert.bugzilla)
Comment on attachment 404900 [details] [diff] [review]
remove plugin files

I missed the ../ in philor's comment and it concerns me a tad. Can you verify that this still works?
FWIW, ../ worked last June (and I would have been testing with 1.9.1), bug 496683 comment 16, but I didn't want to risk it becoming unsupported because Fx didn't use it, so I didn't land my potential use. I gave Markus a link to my comment there explaining how to build and use an updater, though, so he can double-check that he's emptying out all the files and we'll at least know that it works for two people before we land it :)
If this works and we go this route I'll add it to the existing updater binary tests.
I tried to test this today, but something must be wrong with my mozconfig, since the builds that I found in the dmg after "make package" didn't even start... I'll try again tomorrow.
Yes, it works! The relevant part from update.log:

> FINISH REMOVE Contents/MacOS/../Plug-Ins/PrintPDE.plugin/Contents/Info.plist
> FINISH REMOVE Contents/MacOS/../Plug-Ins/PrintPDE.plugin/Contents/MacOS/PrintPDE
> FINISH REMOVE Contents/MacOS/../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/Localizable.strings
> FINISH REMOVE Contents/MacOS/../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/classes.nib
> FINISH REMOVE Contents/MacOS/../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/info.nib
> FINISH REMOVE Contents/MacOS/../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/objects.xib
Attachment #404900 - Flags: review?(robert.bugzilla) → review+
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Attachment #404551 - Flags: approval1.9.2?
Markus, looks like you missed the PrintPDE makefile reference in ;)
Flags: blocking1.9.2+
Priority: P1 → P2
Whiteboard: [crashkill]
Comment on attachment 404551 [details] [diff] [review]
v1, 1.9.2 branch version

Do we need an updated 1.9.2 patch with all the follow-on fixes from this bug?  Re-request approval if not, or with a new and 1.9.2-tested patch if so!
Attachment #404551 - Flags: approval1.9.2?
Whiteboard: [crashkill] → [crashkill][needs 1.9.2 patch]
Pushed to 1.9.2:
This patch also contains the one for bug 523323.
Attachment #404551 - Attachment is obsolete: true
Whiteboard: [crashkill][needs 1.9.2 patch] → [crashkill]
Depends on: 713418
Depends on: 714042
You need to log in before you can comment on or make changes to this bug.