737765 - aLength is not used in insertTextCB()

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Ginn Chen]

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 (:asurkov)]

The length should be explicitly passed into NS_ConvertUTF8toUTF16 constructor

Comment 2

[Charles Chan]

Attachment: Bug 737765: Patch-1

Please review the patch.

Comment 3

[Hubert Figuiere [:hub]]

Comment on attachment 608965 [details] [diff] [review]
Bug 737765: Patch-1

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

looks good.

Comment 4

[Charles Chan]

Thanks, Hub. 

As I don't have checkin access, could you please kindly do so for me?

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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]

> Is it possible to write a test for this?
Of course! Should I be looking under accessible/tests/mochitest/?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Sounds like the right spot, yes. Thanks!

Comment 8

[Trevor Saunders (:tbsaunde)]

> 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]

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)]

(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]]

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 (:asurkov)]

(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]]

Is it possible to create an automated test for our testsuite?

Comment 14

[alexander :surkov (:asurkov)]

(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]]

Just going through my request queue. I have a lot of in-testsuite? requests in it :)

Comment 16

[alexander :surkov (:asurkov)]

this one may be not so important, so we could denominate it