374362 - Build failure with -enable-accessibility, using OSX10.3.9SDK

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[philippe (part-time)]

Build environment 10.4.9 PPC, latest Xcode.
Building Minefield/Camino with ac_add_options --enable-accessibility results in failure (using GCC 3.3, MacOSX10.3.9.sdk):

camino-build/mozilla/accessible/src/mac/mozAccessible.mm
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozAccessible.mm: In 
   function `objc_object* -[mozAccessible 
   accessibilityAttributeValue:](mozAccessible*, objc_selector*, NSString*)':
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozAccessible.mm:205: error: `
   NSAccessibilityRoleDescription' undeclared (first use this function)
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozAccessible.mm:205: error: (Each
   undeclared identifier is reported only once for each function it appears 
   in.)
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozAccessible.mm:206: warning: invalid
   conversion from `const NSString*' to `NSString*'
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozAccessible.mm:214: warning: invalid
   conversion from `const NSString*' to `NSString*'
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozAccessible.mm: In 
   function `NSString* -[mozAccessible role](mozAccessible*, objc_selector*)':
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozAccessible.mm:417: warning: invalid
   conversion from `const NSString*' to `NSString*'
make[6]: *** [mozAccessible.o] Error 1
...

Using GCC 4.01+MacOSX10.3.9.sdk fails with the same error. 
Only using the MacOSX10.4u.sdk results in a successful build.

Per a comment from smorgan in the forums:
"NSAccessibilityRoleDescription is 10.4+, so there's no need to test gcc 3.3 or gather build logs. That function needs to be weak-linked or removed in order for the accessibility code to build for 10.3.9."
http://forums.mozillazine.org/viewtopic.php?p=2801252#2801252

Expected: successful build.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(Standard Mac OS X PPC builds require the 10.3.9 SDK on the trunk, so this will cause Mac tinderboxen to go red if it's not fixed before --enable-accessibility its on by default on Mac OS X.)

Comment 2

[Aaron Leventhal]

This is probably surkov.

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch

not sure it's my bad. I tried to fix it and if I get the bug right then it should work. Though I'm not sure what should be if the given attribute is NSAccessibilityRoleDescriptionAttribute. If mac guys know how it should be then please let me know.

Comment 4

[alexander :surkov (:asurkov)]

Please test the patch.

Comment 5

[philippe (part-time)]

Next error... 
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozActionElements.mm: In function 'NSArray* -[mozPopupButtonAccessible accessibilityActionNames](mozPopupButtonAccessible*, objc_selector*)':
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozActionElements.mm:188: error: 'NSAccessibilityShowMenuAction' was not declared in this scope
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozActionElements.mm: In function 'NSString* -[mozPopupButtonAccessible accessibilityActionDescription:](mozPopupButtonAccessible*, objc_selector*, NSString*)':
/Users/phiw/camino-build/mozilla/accessible/src/mac/mozActionElements.mm:196: error: 'NSAccessibilityShowMenuAction' was not declared in this scope
make[6]: *** [mozActionElements.o] Error 1

Seeing this error, I suspect that lots of what Håkan has done need to be checked for compatibility with the OS10.3.9sdk (required to build on/for OS X PowerPC).

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 258955 [details] [diff] [review]
patch

If aaronlev is fine with the patch then I think I can continue slice mac ally support for 1.3.9 version ;)

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> (From update of attachment 258955 [details] [diff] [review])
> If aaronlev is fine with the patch then I think I can continue slice mac ally
> support for 1.3.9 version ;)
> 

Or it will be better if some mac guy can do it.

Comment 8

[Håkan Waara]

Comment on attachment 258955 [details] [diff] [review]
patch

Thanks. (I thought I'd get bugmail for being CC'd but apparently not)

Comment 9

[alexander :surkov (:asurkov)]

(In reply to comment #8)
> (From update of attachment 258955 [details] [diff] [review])
> Thanks. (I thought I'd get bugmail for being CC'd but apparently not)
> 

Thank you, Hekan. Then the approach is right.

Comment 10

[alexander :surkov (:asurkov)]

Attachment: patch2

next portion, please test.

Comment 11

[alexander :surkov (:asurkov)]

philippe, feel free to take the bug because it is a long game to pass patches from me to you.

Comment 12

[philippe (part-time)]

(In reply to comment #10)
> Created an attachment (id=259005) [details]
> patch2
Sorry about the delay, lots of work the past 2 days
Unfortunately, this gives the same build error as commented above:
Users/phiw/cocoafox/mozilla/accessible/src/mac/mozActionElements.mm: In function 'NSString* -[mozPopupButtonAccessible accessibilityActionDescription:](mozPopupButtonAccessible*, objc_selector*, NSString*)':
/Users/phiw/cocoafox/mozilla/accessible/src/mac/mozActionElements.mm:198: error: 'NSAccessibilityShowMenuAction' was not declared in this scope
make[6]: *** [mozActionElements.o] Error 1
make[5]: *** [libs] Error 2

(In reply to comment #11)
> philippe, feel free to take the bug because it is a long game to pass patches
> from me to you.
> 
But, I don't understand most of that code, or any code for the matter.

Comment 13

[Håkan Waara]

Comment on attachment 259005 [details] [diff] [review]
patch2

> - (NSArray *)accessibilityActionNames
> {
>+#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_4
>   if ([self isEnabled]) {
>-    return [NSArray arrayWithObjects:NSAccessibilityPressAction, 
>-                                     NSAccessibilityShowMenuAction, 
>+    return [NSArray arrayWithObjects:NSAccessibilityPressAction,
>+                                     NSAccessibilityShowMenuAction,
>                                      nil];
>   }
>+#endif
>   return nil;
> }


The problem here is that apparently the ShowMenu action is new in 10.4 (see http://tinyurl.com/2xqr49). All constants or NSAccessibility features that are new as of Tiger are marked as such in apple's docs on NSAccessibility.

You should only ifdef out the NSAccessibilityShowMenuAction line, and leave the line above and below as they are (otherwise you can't even invoke a click on popupmenus via accessibility any longer).

Comment 14

[alexander :surkov (:asurkov)]

Yea, I get it. I just was totally blind :)

Comment 15

[alexander :surkov (:asurkov)]

(In reply to comment #12)

> (In reply to comment #11)
> > philippe, feel free to take the bug because it is a long game to pass patches
> > from me to you.
> > 
> But, I don't understand most of that code, or any code for the matter.
> 

In general you shoudn't deeply understand the code. To fix the bug we should if/endif all 10.4 stuff. No more. Than Hakan will look at the patch and will say what's wrong. Though if you prefer the way we are going on then it's fine with me, it's not a problem to look at your report and put additional if/endif :)

Comment 16

[alexander :surkov (:asurkov)]

Attachment: patch3

Comment 17

[philippe (part-time)]

(In reply to comment #16)
> Created an attachment (id=259341) [details]
> patch3
> 

Build succeeded !

Comment 18

[alexander :surkov (:asurkov)]

Comment on attachment 259341 [details] [diff] [review]
patch3

(In reply to comment #17)
> (In reply to comment #16)
> > Created an attachment (id=259341) [details] [details]
> > patch3
> > 
> 
> Build succeeded !
> 

Ok. Thank you for testing. Hekan can review this?

Comment 19

[Håkan Waara]

Comment on attachment 259341 [details] [diff] [review]
patch3

One way to avoid all the ifdefs for 10.4-specific attributes is to define the constants ourselves. I've done this with a few already.

But this approach is OK since the bustage needs to be fixed.

Comment 20

[alexander :surkov (:asurkov)]

(In reply to comment #19)
> (From update of attachment 259341 [details] [diff] [review])
> One way to avoid all the ifdefs for 10.4-specific attributes is to define the
> constants ourselves. I've done this with a few already.
> 
> But this approach is OK since the bustage needs to be fixed.
> 

checked in

Comment 21

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #19)
> (From update of attachment 259341 [details] [diff] [review])
> One way to avoid all the ifdefs for 10.4-specific attributes is to define the
> constants ourselves. I've done this with a few already.
> 
> But this approach is OK since the bustage needs to be fixed.

I filed bug 375564 on doing this the right way ;)

Thanks for the fixes!