Closed
Bug 418105
Opened 16 years ago
Closed 16 years ago
Remove non-cairo Mac gfx code from the tree
Categories
(Core Graveyard :: GFX: Mac, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mozilla, Assigned: mozilla)
References
Details
Attachments
(1 file, 3 obsolete files)
431.38 KB,
patch
|
jaas
:
review+
vlad
:
superreview+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
The whole tree in gfx/src/mac is still there and should be removed, together with references to it. I found references to this directory only in toolkit/toolkit-makefiles.sh and xpfe/bootstrap/appleevents/Makefile.in.
Assignee | ||
Comment 2•16 years ago
|
||
I don't have a mac and so can't really test this but as I am in the mood for more cleanup/removal stuff, here is the patch. Only the first two hunks are changes to files that will stay, the rest is the content of the files to be CVS removed. I found a reference to gfx_mac in embedding/tests/cocoaEmbed/CocoaEmbed.pbproj/project.pbxproj but as that seems to be a generated file I didn't want to touch it. Anything else that I might have missed?
Assignee: joshmoz → mozilla
Status: NEW → ASSIGNED
Attachment #304855 -
Flags: superreview?(vladimir)
Attachment #304855 -
Flags: review?(joshmoz)
Comment on attachment 304855 [details] [diff] [review] remove it and the remaining references Why not also remove nsMacGFX.rsrc? And everything in the printerplugin directory? The current patch doesn't compile because of the following error: mozilla/xpfe/bootstrap/appleevents/nsAEWindowClass.cpp:55:24: error: nsGfxUtils.h: No such file or directory mozilla/xpfe/bootstrap/appleevents/nsAEWindowClass.cpp: In member function ‘void AEWindowClass::SetWindowProperties(OpaqueWindowPtr*, const AEDesc*)’: mozilla/xpfe/bootstrap/appleevents/nsAEWindowClass.cpp:776: error: ‘StPortSetter’ was not declared in this scope mozilla/xpfe/bootstrap/appleevents/nsAEWindowClass.cpp:776: error: expected `;' before ‘portSetter’
Attachment #304855 -
Flags: superreview?(vladimir)
Attachment #304855 -
Flags: review?(joshmoz)
Attachment #304855 -
Flags: review-
Assignee | ||
Comment 4•16 years ago
|
||
(In reply to comment #3) > (From update of attachment 304855 [details] [diff] [review]) > Why not also remove nsMacGFX.rsrc? And everything in the printerplugin > directory? Those are going to be removed. But as they are binary files, they only show up as e.g. "Files gfx/src/mac/nsMacGFX.rsrc and /dev/null differ" in the patch. > The current patch doesn't compile because of the following error: > > mozilla/xpfe/bootstrap/appleevents/nsAEWindowClass.cpp:55:24: error: > nsGfxUtils.h: No such file or directory > mozilla/xpfe/bootstrap/appleevents/nsAEWindowClass.cpp: In member function > ‘void AEWindowClass::SetWindowProperties(OpaqueWindowPtr*, const AEDesc*)’: > mozilla/xpfe/bootstrap/appleevents/nsAEWindowClass.cpp:776: error: > ‘StPortSetter’ was not declared in this scope > mozilla/xpfe/bootstrap/appleevents/nsAEWindowClass.cpp:776: error: expected `;' > before ‘portSetter’ Josh, can you suggest as a fix for this? Moving nsGfxUtils.h to xpfe/bootstrap/appleevents/ doesn't seem like a good solution. Should I move the StPortSetter class definition from nsGfxUtils.h to nsAEWindowClass.h instead? As I don't have a Mac I can't verify that this will work.
Just move stportsetter to nsAEWindowClass and remove the reference from the other file there, which doesn't seem to actually use it.
Assignee | ||
Comment 6•16 years ago
|
||
OK, so this should be better.
Attachment #304855 -
Attachment is obsolete: true
Attachment #306616 -
Flags: superreview?(vladimir)
Attachment #306616 -
Flags: review?(joshmoz)
Assignee | ||
Comment 8•16 years ago
|
||
Ah, sorry. I created this one on OS/2 and the editor must have saved it with DOS linebreaks. (I wonder why they are not transformed to the native format when you download the patch from Bugzilla.) Is there a tool to convert such files to Unix linebreaks on Mac? Otherwise, if you are worried about that, I can upload a new version.
Assignee | ||
Comment 9•16 years ago
|
||
OK, I remembered the try server and uploaded this patch (together with the one for bug418104). It took a few trials to get it right but this one now really builds and uses Unix-type linebreaks. If someone wants to try the resulting try server Mac build, it's in https://build.mozilla.org/tryserver-builds/2008-03-01_07:24-mozilla@weilbacher.org-macwingfxremoval3/
Attachment #306616 -
Attachment is obsolete: true
Attachment #306744 -
Flags: superreview?(vladimir)
Attachment #306744 -
Flags: review?(joshmoz)
Attachment #306616 -
Flags: superreview?(vladimir)
Attachment #306616 -
Flags: review?(joshmoz)
Comment 10•16 years ago
|
||
+// the following was moved here from old Mac GFX headers This comment is useless, please remove it. +inline PRBool CurrentPortIsWMPort() +{ + return PR_FALSE; +} What is the point of this? Just remove that and fix up the logic that uses it. You can remove that function and this code that uses it since it'll never be true: + // if we have a window, but we're set to the WM port, things are bad + if (CurrentPortIsWMPort() && (FrontWindow() != nil)) + return false; Otherwise this compiles and works fine, thanks. Please post a new patch with that stuff taken care of.
Assignee | ||
Comment 11•16 years ago
|
||
(In reply to comment #10) > +// the following was moved here from old Mac GFX headers > > This comment is useless, please remove it. Well, it would tell people where the code that I'm simply moving from somewhere else comes from, so that they could do more CVS archeology. In my experience this is not easily apparent otherwise. But OK, I personally will never again look at this, so I'll do whatever you want...
Assignee | ||
Comment 12•16 years ago
|
||
Comments addressed.
Attachment #306744 -
Attachment is obsolete: true
Attachment #306925 -
Flags: review?(joshmoz)
Attachment #306744 -
Flags: superreview?(vladimir)
Attachment #306744 -
Flags: review?(joshmoz)
Comment 13•16 years ago
|
||
Comment on attachment 306925 [details] [diff] [review] v4 Thanks, it is nice to have this cleaned up. Makes things less confusing for potential contributors and cleans up mxr results.
Attachment #306925 -
Flags: superreview?(vladimir)
Attachment #306925 -
Flags: review?(joshmoz)
Attachment #306925 -
Flags: review+
Assignee | ||
Comment 14•16 years ago
|
||
Vladimir, will you have time to sr this? If not who else should? This blocks a wanted-1.9 bug, so I would like to get it in before 1.9b5.
Attachment #306925 -
Flags: superreview?(vladimir) → superreview+
Attachment #306925 -
Flags: approval1.9?
Comment 15•16 years ago
|
||
Comment on attachment 306925 [details] [diff] [review] v4 a1.9=beltzner
Attachment #306925 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 16•16 years ago
|
||
Changes checked into trunk and all files under gfx/src/mac cvs removed.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•