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

Status: RESOLVED FIXED

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

Description

[Markus Stange [:mstange]]

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.

Comment 1

[Markus Stange [:mstange]]

Attachment: 10.4 changes

Comment 2

[Markus Stange [:mstange]]

Attachment: backout bug 456646

This is just a backout of bug 456646, provided for easier mq-ability.

Comment 3

[Markus Stange [:mstange]]

Attachment: v1

New trunk patch with 10.4 support, applies on top of the backout.

Comment 4

[Markus Stange [:mstange]]

Attachment: v1, 1.9.2 branch version

Like the trunk patch, just without #ifdefs and without any changes to *.properties files (because 1.9.2 is already string-frozen).

Comment 5

[Josh Aas]

Comment on attachment 404550 [details] [diff] [review]
v1

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

This is not used.

Comment 6

[Markus Stange [:mstange]]

Argh, I thought I had removed that again.

Comment 7

[Markus Stange [:mstange]]

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 removed-files.in? 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/PrintPDE.build/Development/PrintPDE.build/Objects-normal/i386/PDECore.o
> embedding/components/printingui/src/mac/printpde/build/PrintPDE.build/Development/PrintPDE.build/Objects-normal/i386/PDECustom.o
> embedding/components/printingui/src/mac/printpde/build/PrintPDE.build/Development/PrintPDE.build/Objects-normal/i386/PDEUtilities.o
> embedding/components/printingui/src/mac/printpde/build/PrintPDE.build/Development/PrintPDE.build/Objects-normal/i386/PrintPDE.LinkFileList
> embedding/components/printingui/src/mac/printpde/build/PrintPDE.build/Development/PrintPDE.build/PrintPDE.dep
> embedding/components/printingui/src/mac/printpde/build/PrintPDE.build/Development/PrintPDE.build/PrintPDE.hmap
> embedding/components/printingui/src/mac/printpde/build/PrintPDE.build/Development/PrintPDE.build/PrintPDE~.dep

Comment 8

[Phil Ringnalda (:philor)]

Not that set, no: removed-files.in works against the contents of Firefox.app, not an objdir. Think of it as a script running from Firefox.app/Contents/MacOS/. So according to |find ../Plug-Ins -type f| in the first random Firefox.app I found, it would be

../Plug-Ins/PrintPDE.plugin/Contents/Info.plist
../Plug-Ins/PrintPDE.plugin/Contents/MacOS/PrintPDE
../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/Localizable.strings
../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/classes.nib
../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/info.nib
../Plug-Ins/PrintPDE.plugin/Contents/Resources/English.lproj/PrintPDE.nib/objects.xib

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

* We've never actually used ../ in removed-files.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?

Comment 9

[(not currently active) Ted Mielczarek]

FWIW, I don't really know how removed-files.in works at a functional level. I'd take philor's word for it. CCing Rob Strong for more insight.

Comment 10

[Robert Strong (they/them - no direct email)]

Philor has it right. It is a list of files to remove if they exist on app update.
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/src/updater/updater.cpp#633

Comment 11

[Markus Stange [:mstange]]

Attachment: remove plugin files

Comment 12

[Robert Strong (they/them - no direct email)]

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?

Comment 13

[Phil Ringnalda (:philor)]

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 :)

Comment 14

[Robert Strong (they/them - no direct email)]

If this works and we go this route I'll add it to the existing updater binary tests.

Comment 15

[Markus Stange [:mstange]]

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.

Comment 16

[Markus Stange [:mstange]]

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

Comment 17

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/8d2de8ddfa3b
http://hg.mozilla.org/mozilla-central/rev/1d371a457f5d
http://hg.mozilla.org/mozilla-central/rev/74f8b81bdbea

Comment 18

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Markus, looks like you missed the PrintPDE makefile reference in toolkik-makefiles.sh ;)

http://mxr.mozilla.org/mozilla-central/source/toolkit/toolkit-makefiles.sh#644

Comment 19

[Phil Ringnalda (:philor)]

Whacked that last mole in http://hg.mozilla.org/mozilla-central/rev/a6f0d32ba6a3

Comment 20

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

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!

Comment 21

[Markus Stange [:mstange]]

Attachment: updated 1.9.2 patch

Pushed to 1.9.2: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7fa0282e8709
This patch also contains the one for bug 523323.