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

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Core
Widget: Cocoa
P2
normal
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

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

Firefox Tracking Flags

(status1.9.2 beta2-fixed)

Details

(Whiteboard: [crashkill], URL)

Attachments

(5 attachments, 1 obsolete attachment)

(Assignee)

Description

9 years ago
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.
(Assignee)

Comment 1

9 years ago
Created attachment 404535 [details] [diff] [review]
10.4 changes
(Assignee)

Comment 2

9 years ago
Created attachment 404549 [details] [diff] [review]
backout bug 456646

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

Comment 3

9 years ago
Created attachment 404550 [details] [diff] [review]
v1

New trunk patch with 10.4 support, applies on top of the backout.
Attachment #404550 - Flags: review?(joshmoz)
(Assignee)

Comment 4

9 years ago
Created attachment 404551 [details] [diff] [review]
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).
Blocks: 520394

Updated

9 years ago
Attachment #404550 - Flags: review?(joshmoz) → review+

Comment 5

9 years ago
Comment on attachment 404550 [details] [diff] [review]
v1

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

This is not used.
(Assignee)

Comment 6

9 years ago
Argh, I thought I had removed that again.
(Assignee)

Comment 7

9 years ago
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.
(Assignee)

Comment 11

9 years ago
Created attachment 404900 [details] [diff] [review]
remove plugin files
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.
(Assignee)

Comment 15

9 years ago
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.
(Assignee)

Comment 16

9 years ago
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+
(Assignee)

Comment 17

9 years ago
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
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
(Assignee)

Updated

9 years ago
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

Updated

9 years ago
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?
(Assignee)

Updated

9 years ago
Whiteboard: [crashkill] → [crashkill][needs 1.9.2 patch]
(Assignee)

Comment 21

9 years ago
Created attachment 410529 [details] [diff] [review]
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.
Attachment #404551 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
status1.9.2: --- → final-fixed
Whiteboard: [crashkill][needs 1.9.2 patch] → [crashkill]
(Assignee)

Updated

8 years ago
Duplicate of this bug: 420924
Depends on: 713418
Depends on: 714042
You need to log in before you can comment on or make changes to this bug.