Closed
Bug 520494
Opened 15 years ago
Closed 15 years ago
Implement the Cocoa print dialog in a way that works on 10.4
Categories
(Core :: Widget: Cocoa, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta2-fixed |
People
(Reporter: mstange, Assigned: mstange)
References
()
Details
(Whiteboard: [crashkill])
Attachments
(5 files, 1 obsolete file)
17.84 KB,
patch
|
Details | Diff | Splinter Review | |
77.54 KB,
patch
|
Details | Diff | Splinter Review | |
237.40 KB,
patch
|
jaas
:
review+
|
Details | Diff | Splinter Review |
1.21 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
225.66 KB,
patch
|
Details | Diff | Splinter Review |
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•15 years ago
|
||
Assignee | ||
Comment 2•15 years ago
|
||
This is just a backout of bug 456646, provided for easier mq-ability.
Assignee | ||
Comment 3•15 years ago
|
||
New trunk patch with 10.4 support, applies on top of the backout.
Attachment #404550 -
Flags: review?(joshmoz)
Assignee | ||
Comment 4•15 years ago
|
||
Like the trunk patch, just without #ifdefs and without any changes to *.properties files (because 1.9.2 is already string-frozen).
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.
Assignee | ||
Comment 6•15 years ago
|
||
Argh, I thought I had removed that again.
Assignee | ||
Comment 7•15 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
Comment 8•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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
Assignee | ||
Comment 11•15 years ago
|
||
Attachment #404900 -
Flags: review?(robert.bugzilla)
Comment 12•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
If this works and we go this route I'll add it to the existing updater binary tests.
Assignee | ||
Comment 15•15 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•15 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
Updated•15 years ago
|
Attachment #404900 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 17•15 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
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Assignee | ||
Updated•15 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
Comment 19•15 years ago
|
||
Whacked that last mole in http://hg.mozilla.org/mozilla-central/rev/a6f0d32ba6a3
Updated•15 years ago
|
Flags: blocking1.9.2+
Priority: P1 → P2
Whiteboard: [crashkill]
Comment 20•15 years ago
|
||
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•15 years ago
|
Whiteboard: [crashkill] → [crashkill][needs 1.9.2 patch]
Assignee | ||
Comment 21•15 years ago
|
||
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•15 years ago
|
status1.9.2:
--- → final-fixed
Whiteboard: [crashkill][needs 1.9.2 patch] → [crashkill]
You need to log in
before you can comment on or make changes to this bug.
Description
•