Closed
Bug 824927
Opened 11 years ago
Closed 11 years ago
Accessibility check missing in b2g configure
Categories
(Firefox OS Graveyard :: General, defect)
Firefox OS Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: prabu, Assigned: prabu)
References
Details
Attachments
(1 file, 2 obsolete files)
1.15 KB,
patch
|
prabu
:
review+
|
Details | Diff | 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
Comment 3•11 years ago
|
||
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-
Updated•11 years ago
|
Assignee: nobody → prabu
Updated per review comments
Attachment #695991 -
Attachment is obsolete: true
Attachment #697785 -
Flags: review?(ted)
Comment 5•11 years ago
|
||
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+
Comment 7•11 years ago
|
||
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
Attachment #697785 -
Attachment is obsolete: true
Attachment #697956 -
Flags: review+
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/870dca5f3fdc
Keywords: checkin-needed
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/870dca5f3fdc
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•