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

RESOLVED FIXED in mozilla1.9.3a1

Status

()

defect
P2
normal
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

Trunk
mozilla1.9.3a1
All
macOS
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta2-fixed)

Details

(Whiteboard: [crashkill], )

Attachments

(5 attachments, 1 obsolete attachment)

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.
Posted patch 10.4 changesSplinter Review
This is just a backout of bug 456646, provided for easier mq-ability.
Posted patch v1Splinter Review
New trunk patch with 10.4 support, applies on top of the backout.
Attachment #404550 - Flags: review?(joshmoz)
Posted 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]
v1

+- (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 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
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?
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.
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+
http://hg.mozilla.org/mozilla-central/rev/8d2de8ddfa3b
http://hg.mozilla.org/mozilla-central/rev/1d371a457f5d
http://hg.mozilla.org/mozilla-central/rev/74f8b81bdbea
Status: ASSIGNED → RESOLVED
Closed: 10 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 toolkik-makefiles.sh ;)

http://mxr.mozilla.org/mozilla-central/source/toolkit/toolkit-makefiles.sh#644
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: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7fa0282e8709
This patch also contains the one for bug 523323.
Attachment #404551 - Attachment is obsolete: true
Whiteboard: [crashkill][needs 1.9.2 patch] → [crashkill]
Duplicate of this bug: 420924
Depends on: 713418
You need to log in before you can comment on or make changes to this bug.