Closed
Bug 151388
Opened 22 years ago
Closed 22 years ago
To add a new compile switch to distinguish Windows and ATK code
Categories
(SeaMonkey :: Build Config, defect)
SeaMonkey
Build Config
Tracking
(Not tracked)
CLOSED
FIXED
People
(Reporter: yuanyi21, Assigned: john.sun)
References
Details
Attachments
(1 file, 2 obsolete files)
547 bytes,
patch
|
netscape
:
review+
|
Details | Diff | Splinter Review |
So that we can use #ifdef ACCESSIBILITY_ATK //ATK code here #else //Windows code here #endif to keep the binaries smaller.
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.
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 )
Status: NEW → ASSIGNED
IMO, MOZ_ACCESSIBILITY_ATK = XP_UNIX && ACCESSIBILITY && MOZ_ENABLE_GTK2
Seek r=? Just add a macro MOZ_ACCESSIBILITY_ATK in mozilla/configure for accessbility work. Don't affect anything else.
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•22 years ago
|
||
I think Chris Seawood (IRC cls, seawood@netscape.com)
Updated•22 years ago
|
Attachment #89372 -
Flags: needs-work+
Comment 9•22 years ago
|
||
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.
Assignee | ||
Comment 10•22 years ago
|
||
Thank kyle for your reminding. autoconf make configure.in -> configure. seek r=
Attachment #89372 -
Attachment is obsolete: true
Updated•22 years ago
|
Attachment #89382 -
Flags: needs-work+
Comment 11•22 years ago
|
||
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).
Assignee | ||
Comment 12•22 years ago
|
||
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.
Component: Accessibility APIs → Build Config
Comment 13•22 years ago
|
||
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.
Assignee | ||
Comment 14•22 years ago
|
||
Chris, You are right. Thanks for you clarification. seek you review again.
Attachment #89382 -
Attachment is obsolete: true
Comment 15•22 years ago
|
||
Comment on attachment 89504 [details] [diff] [review] V3 r=cls
Attachment #89504 -
Flags: review+
Comment 16•22 years ago
|
||
checked in
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 18•22 years ago
|
||
John, you should use VERIFIED instead of CLOSED.
Assignee | ||
Comment 19•22 years ago
|
||
Thank Peter and Kyle's reminding, I'll do as what you said for other bugs.
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•