remove non-cairo gfx support from cocoa widgets

RESOLVED FIXED

Status

()

Core
Widget: Cocoa
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Josh Aas, Assigned: Josh Aas)

Tracking

Trunk
PowerPC
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

38.42 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
Within a few weeks gecko will remove support for building without cairo gfx. Camino already doesn't support non-cairo gfx. We have a lot of clutter in cocoa widgets because of supporting non-cairo gfx, I think it is time to remove it.
(Assignee)

Comment 1

11 years ago
Created attachment 249143 [details] [diff] [review]
fix v1.0
Attachment #249143 - Flags: review?(stuart.morgan)
(Assignee)

Comment 2

11 years ago
This is already messing with enn in 178513.
(Assignee)

Comment 3

11 years ago
By messing with enn, I mean supporting non-cairo gfx is problematic for him.

Comment 4

11 years ago
Comment on attachment 249143 [details] [diff] [review]
fix v1.0

> 		$(MAC_LCPPSRCS) \
>+    nsNativeThemeCocoa.cpp \
> 		$(NULL)

This should use tabs, since it's a makefile.

> EXTRA_DSO_LDOPTS += \
> 		$(MOZ_UNICHARUTIL_LIBS) \
> 		$(TK_LIBS) \
> 		$(call EXPAND_LIBNAME_PATH,gkgfx,$(DEPTH)/gfx/src) \
> 		$(MOZ_COMPONENT_LIBS) \
> 		$(NULL)
> 
>-ifdef MOZ_ENABLE_CAIRO_GFX
> EXTRA_DSO_LDOPTS += -lthebes
>-endif

Can this just go inline in the list above, rather than being added after?

>-// Depending on whether we're on cairo, and if accessibility is on, we support different @protocols

The accessibility part of this comment should stay.


r=smorgan; I won't miss it.
Attachment #249143 - Flags: review?(stuart.morgan) → review+
(Assignee)

Updated

11 years ago
Attachment #249143 - Flags: review?(mark)

Comment 5

11 years ago
Comment on attachment 249143 [details] [diff] [review]
fix v1.0

In addition to what smorgan wrote:

>Index: Makefile.in

>-ifdef MOZ_ENABLE_CAIRO_GFX
> REQUIRES += thebes cairo

These can be added to the REQUIRES list above.
Attachment #249143 - Flags: review?(mark) → review+
(Assignee)

Updated

11 years ago
Attachment #249143 - Flags: superreview?(mikepinkerton)
Comment on attachment 249143 [details] [diff] [review]
fix v1.0

rs=pink
Attachment #249143 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 7

11 years ago
Created attachment 249144 [details] [diff] [review]
fix v1.0.1

This is what I plan to land, fixes based on review comments.
Attachment #249143 - Attachment is obsolete: true
(Assignee)

Comment 8

11 years ago
landed on trunk
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.