824927 - Accessibility check missing in b2g configure

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[prabindh]

Attachment: configure_accessibility.patch

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:10.0.1) Gecko/20100101 Firefox/10.0.1
Build ID: 20120209214325

Steps to reproduce:

Building b2g- on ARM linux, with --disable-accessibility in mozconfig. Since Accessfu.jsm is not packaged into the target dist, no rendering happens.


Actual results:

Since Accessfu.jsm is not packaged into the target dist, no rendering happens on screen


Expected results:

Accessfu.jsm should have been packaged for the target runtime, and page rendered on the screen.

Comment 1

[prabindh]

This bug was created based on suggestion from :mphil at https://bugzilla.mozilla.org/show_bug.cgi?id=731498

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 695991 [details] [diff] [review]
configure_accessibility.patch

Review of attachment 695991 [details] [diff] [review]:
-----------------------------------------------------------------

I was going to r+ since these are mostly just nits, but the nit-to-patch ratio is a bit high. Fix up these few minor things and it should be an easy r+.

::: configure.in
@@ +5241,5 @@
>  fi
>  
>  dnl ========================================================
> +dnl Atleast linuxgl widget does not work with 
> +dnl   --disable-accessibility as AccessFu.jsm is not 

This comment could just read "Accessibility is required for the linuxgl widget backend". Also you have some trailing spaces on these comment lines.

@@ +5245,5 @@
> +dnl   --disable-accessibility as AccessFu.jsm is not 
> +dnl packaged in
> +dnl ========================================================
> +if test "${MOZ_WIDGET_TOOLKIT}" = "linuxgl"; then
> +  if test "$ACCESSIBILITY" != "1"; then

You could just combine these into a single test:
if test "${MOZ_WIDGET_TOOLKIT}" = "linuxgl" -a "$ACCESSIBILITY" != "1"; then

@@ +5246,5 @@
> +dnl packaged in
> +dnl ========================================================
> +if test "${MOZ_WIDGET_TOOLKIT}" = "linuxgl"; then
> +  if test "$ACCESSIBILITY" != "1"; then
> +        AC_MSG_ERROR([Accessibility is disabled, but needed for AccessFu to work])

"Accessibility is required for the linuxgl widget backend."

@@ +9192,5 @@
>  AC_OUTPUT_SUBDIRS(js/src)
>  ac_configure_args="$_SUBDIR_CONFIG_ARGS"
>  
>  fi # COMPILE_ENVIRONMENT && !LIBXUL_SDK_DIR
> +

Please don't add this extraneous blank line.

Comment 4

[prabindh]

Attachment: Updated accessibility check in configure for linuxgl - per review comments

Updated per review comments

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 697785 [details] [diff] [review]
Updated accessibility check in configure for linuxgl - per review comments

Review of attachment 697785 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 6

[prabindh]

Ted, what happens next ? How do I get this to commit ?

Comment 7

[(not currently active) Ted Mielczarek]

The easiest way is to add the checkin-needed keyword and wait for the checkin fairies to land it for you. You might want to read some notes on that, though:
https://developer.mozilla.org/en-US/docs/Creating_a_patch_that_can_be_checked_in

Comment 8

[prabindh]

Attachment: Patch formatted for checkin-needed

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/870dca5f3fdc

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/870dca5f3fdc