Closed Bug 824927 Opened 7 years ago Closed 7 years ago

Accessibility check missing in b2g configure

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: prabu, Assigned: prabu)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch configure_accessibility.patch (obsolete) — Splinter Review
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.
Attachment #695991 - Flags: review?(ted)
This bug was created based on suggestion from :mphil at https://bugzilla.mozilla.org/show_bug.cgi?id=731498
Duplicate of this bug: 823985
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.
Attachment #695991 - Flags: review?(ted) → review-
Assignee: nobody → prabu
Updated per review comments
Attachment #695991 - Attachment is obsolete: true
Attachment #697785 - Flags: review?(ted)
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!
Attachment #697785 - Flags: review?(ted) → review+
Ted, what happens next ? How do I get this to commit ?
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: checkin-needed
Attachment #697785 - Attachment is obsolete: true
Attachment #697956 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/870dca5f3fdc
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.