151388 - To add a new compile switch to distinguish Windows and ATK code

Status: CLOSED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Kyle Yuan]

So that we can use 

#ifdef ACCESSIBILITY_ATK
//ATK code here
#else
//Windows code here
#endif

to keep the binaries smaller.

Comment 1

[timeless]

yuck. no no no.

what is wrong with #ifdef XP_WIN ??

Comment 2

[Kyle Yuan]

timeless, we need this switch not only in accessibility code, but also in other 
modules, see:
http://lxr.mozilla.org/seamonkey/source/layout/html/forms/src/nsListControlFrame
.cpp#1478

However, #ifdef XP_WIN is enough for our current developing. We can decide 
whether need that switch in the future.

Comment 3

[John Sun]

Pls refer to http://bugzilla.mozilla.org/show_bug.cgi?id=153816
I'll add a macro MOZ_ACCESSIBILITY_ATK in mozilla/configure.
Its preconditions are XP_UNIX and MOZ_ENABLE_GTK2 defined. It make sense?
( XP_UNIX don't include BeOS and OS2 )

Comment 4

[Kyle Yuan]

IMO, MOZ_ACCESSIBILITY_ATK = XP_UNIX && ACCESSIBILITY && MOZ_ENABLE_GTK2

Comment 5

[John Sun]

execuse me, even forget ACCESSIBILITY. 

Comment 6

[John Sun]

Attachment: patch-v1

Seek r=?

Just add a macro MOZ_ACCESSIBILITY_ATK in mozilla/configure for accessbility
work.
Don't affect anything else.

Comment 7

[Kyle Yuan]

Is mozilla/configure generated by mozilla/configure.in? someone told me that, 
but I forgot.
aaron, who is the right person to r= this patch?

Comment 8

[Aaron Leventhal]

I think Chris Seawood (IRC cls, [e-mail])

Comment 9

[hacker formerly known as [e-mail]]

Comment on attachment 89372 [details] [diff] [review]
patch-v1

The patch should be against configure.in, not configure.  It seems wrong that
there are 3 different places affected by adding a AC_DEFINE() but I 'll wait
for the patch against configure.in to be certain.

Comment 10

[John Sun]

Attachment: V2

Thank kyle for your reminding.
autoconf make configure.in -> configure.

seek r=

Comment 11

[hacker formerly known as [e-mail]]

Comment on attachment 89382 [details] [diff] [review]
V2

Why recreate those specific platform tests just for this define?  About 10
lines up, there's a if-else tree that does what you're appearing to do here,
namely isolate the "unix" case.  And you missed an OpenVMS check.  I'm not sure
if the platform tests are necessary as gtk2 is only used on unix systems (so
far).

Comment 12

[John Sun]

Chris,

Because you don't like add AC_DEFINE in 3 different place(see commment #9),
I merge them to one place and have to re-test the platform. 
My platform test is equal to that done for XP_UNIX. Since I don't know how to
check if a marco define in AC_DEFINE, use a long check expression.

although gtk2 is not only used in unix system, mozilla don't use gtk2 as a base of
rendering in BeOS, WINNT and OS2, so our accessiblity featurev based on ATK will
invalid. 

Comment 13

[hacker formerly known as [e-mail]]

When you say that "Its preconditions are XP_UNIX and MOZ_ENABLE_GTK2 defined",
are you assuming that a platform that defines XP_UNIX is building using X11? 
That is an incorrect assumption.  XP_UNIX is also set on OSX & OpenVMS which do
not use gtk2 nor any other X11 toolkit by default.  You should stick with the
feature checks and just set MOZ_ACCESSIBILITY_GTK if ACCESSIBILITY & GTK2 are
enabled.  I don't understand where the XP_UNIX dependency comes in if it is not
used to assume X11.

Comment 14

[John Sun]

Attachment: V3

Chris,
You are right. Thanks for you clarification.

seek you review again.

Comment 15

[hacker formerly known as [e-mail]]

Comment on attachment 89504 [details] [diff] [review]
V3

r=cls

Comment 16

[Pete Zha]

checked in

Comment 17

[John Sun]

Thanks  peter's help.
close the bug.

Comment 18

[Kyle Yuan]

John, you should use VERIFIED instead of CLOSED.

Comment 19

[John Sun]

Thank Peter and Kyle's reminding, I'll do as what you said for other bugs.