Last Comment Bug 737765 - aLength is not used in insertTextCB()
: aLength is not used in insertTextCB()
Status: RESOLVED FIXED
[good first bug][mentor=hub@mozilla.c...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: x86 Linux
: -- normal (vote)
: mozilla14
Assigned To: Charles Chan
:
Mentors:
Depends on:
Blocks: cleana11y
  Show dependency treegraph
 
Reported: 2012-03-20 23:39 PDT by Ginn Chen
Modified: 2012-11-10 18:04 PST (History)
5 users (show)
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Bug 737765: Patch-1 (1.09 KB, patch)
2012-03-23 21:49 PDT, Charles Chan
hub: review+
Details | Diff | Review

Description Ginn Chen 2012-03-20 23:39:36 PDT
http://mxr.mozilla.org/mozilla-central/ident?i=insertTextCB

So it doesn't conform the ATK atk_editable_text_insert_text() API, and it may cause read out of boundary if the string passing in is not null terminated.
Comment 1 alexander :surkov 2012-03-21 00:14:48 PDT
The length should be explicitly passed into NS_ConvertUTF8toUTF16 constructor
Comment 2 Charles Chan 2012-03-23 21:49:27 PDT
Created attachment 608965 [details] [diff] [review]
Bug 737765: Patch-1

Please review the patch.
Comment 3 Hubert Figuiere [:hub] 2012-03-23 22:08:31 PDT
Comment on attachment 608965 [details] [diff] [review]
Bug 737765: Patch-1

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

looks good.
Comment 4 Charles Chan 2012-03-24 07:40:38 PDT
Thanks, Hub. 

As I don't have checkin access, could you please kindly do so for me?
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-03-24 15:09:30 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec81d6124138

Is it possible to write a test for this?

Hub, you currently aren't listed as an a11y peer. You should probably fix that :)
https://wiki.mozilla.org/Modules/Core
Comment 6 Charles Chan 2012-03-24 16:37:10 PDT
> Is it possible to write a test for this?
Of course! Should I be looking under accessible/tests/mochitest/?
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-03-24 16:40:45 PDT
Sounds like the right spot, yes. Thanks!
Comment 8 Trevor Saunders (:tbsaunde) 2012-03-24 17:31:45 PDT
> Is it possible to write a test for this?

not currently, (the short story is its only callable from c++), but eventually we should have a test harness to test these apis.  Feel free to leave in-testsuite? if you like, but be aware it will not be fixed for a while.

> Hub, you currently aren't listed as an a11y peer. You should probably fix
> that :)

well, last time I new Alex hadn't made him one, but I think Alex was close enough to asking him to review so I think its fine :)
> https://wiki.mozilla.org/Modules/C
Comment 9 Charles Chan 2012-03-24 17:52:01 PDT
Since Mochitests only supports Javascripts and these functions are not exposed. Should I be looking into 'Compiled-code automated tests' instead?
https://developer.mozilla.org/en/Compiled-code_automated_tests
Comment 10 Trevor Saunders (:tbsaunde) 2012-03-24 18:06:32 PDT
(In reply to Charles Chan from comment #9)
> Since Mochitests only supports Javascripts and these functions are not
> exposed. Should I be looking into 'Compiled-code automated tests' instead?
> https://developer.mozilla.org/en/Compiled-code_automated_tests

no, don't worry about writing tests :)
Comment 11 Ed Morley [:emorley] 2012-03-25 06:29:13 PDT
https://hg.mozilla.org/mozilla-central/rev/ec81d6124138

Congratulations on your first patch in the tree! Hope to see you on IRC (http://irc.mozilla.org/) in #developers soon. 

If you'd like to fix another bug (we'd love it if you did!) but need some inspiration, pop on & say hi - and we'll find something for you :-)
Comment 12 alexander :surkov 2012-03-25 21:59:28 PDT
(In reply to Trevor Saunders (:tbsaunde) from comment #8)
> > Hub, you currently aren't listed as an a11y peer. You should probably fix
> > that :)
> 
> well, last time I new Alex hadn't made him one,

well, I don't make peers but I can do nominations, actually as any other peer.

> but I think Alex was close
> enough to asking him to review so I think its fine :)

that's right
Comment 13 Ryan VanderMeulen [:RyanVM] 2012-11-10 11:15:43 PST
Is it possible to create an automated test for our testsuite?
Comment 14 alexander :surkov 2012-11-10 17:58:33 PST
(In reply to Ryan VanderMeulen from comment #13)
> Is it possible to create an automated test for our testsuite?

No until MATS project is finished. Why do you care about this specific bug?
Comment 15 Ryan VanderMeulen [:RyanVM] 2012-11-10 18:00:10 PST
Just going through my request queue. I have a lot of in-testsuite? requests in it :)
Comment 16 alexander :surkov 2012-11-10 18:04:03 PST
this one may be not so important, so we could denominate it

Note You need to log in before you can comment on or make changes to this bug.