610568 - mochitest with extra whitespaces for nsIAccessibleText

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Fernando Herrera]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/534.10 (KHTML, like Gecko) Chrome/8.0.552.28 Safari/534.10
Build Identifier: 

Add tests with extra whitespaces and punctuation signs for nsIAccessibleText getText functions.

Reproducible: Always

Comment 1

[Fernando Herrera]

Attachment: patch1

Initial bunch of tests. Still need to add tests for divs where the string is collapsed

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> Initial bunch of tests. Still need to add tests for divs where the string is
> collapsed

use pre tag

Comment 3

[alexander :surkov (:asurkov)]

perhaps you will hate me but can we get smaller test string?

      // __B__r__a__v__e__ __ __S__i__r__ __ __ __R__o__b__i__n__ __ __ __ __ __ __r__a__n__ __ __a__w__a__y__.__.__.__ __b__r__a__v__e__l__y__ 

24 	

      //  0  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44

or could you describe what kind of special test it allows us?

Comment 4

[Fernando Herrera]

Well, I wanted to test different amounts of whitespaces (2,3,5,2,1) and also the usage of "..." extra characters.

Comment 5

[Fernando Herrera]

Attachment: patch2

Updated patch extending test to div and editable elements inside a <pre>

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #4)
> Well, I wanted to test different amounts of whitespaces (2,3,5,2,1) and also
> the usage of "..." extra characters.

Ok I agree it might make sense to test 1, 2, 3. But what's about 5, why it is special?

Comment 7

[alexander :surkov (:asurkov)]

Btw, does ATK spec have definition of 'word' term? I wonder about 'away...' where's the word here.

Comment 8

[alexander :surkov (:asurkov)]

and btw why do you need to test '...' boundaries here? In general I would like to see small test string as possible.

Comment 9

[Fernando Herrera]

ok, so let keep this one just for single vs. multiple whitespaces and I'll add another one testing words.

So, what about using this string:
__B__r__a__v__e__ __S__i__r__ __ __R__o__b__i__n__ __ __ __r__a__n
  0  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 21

Comment 10

[Fernando Herrera]

Should we also skip line boundaries testing here or is there any reason to keep them?

Comment 11

[alexander :surkov (:asurkov)]

(In reply to comment #9)
> ok, so let keep this one just for single vs. multiple whitespaces and I'll add
> another one testing words.
> 
> So, what about using this string:
> __B__r__a__v__e__ __S__i__r__ __ __R__o__b__i__n__ __ __ __r__a__n
>   0  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 21

sounds good till it fits to 80 lines restriction ;)

(In reply to comment #10)
> Should we also skip line boundaries testing here or is there any reason to keep
> them?

I think we could skip'em. I don't see any peculiarities they could bring.

Comment 12

[Fernando Herrera]

Attachment: patch3

Updated patch with new test string

Comment 13

[alexander :surkov (:asurkov)]

Comment on attachment 491599 [details] [diff] [review]
patch3


>+  <title>nsIAccessibleText getText related function tests with whitespaces for html:input,html:div and html:textarea</title>

perhaps something like that?
getText... methods tests on string with whitespaces for plain text containers

>+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css" />

put @href on new line please

>+  <script type="application/javascript"
>+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
>+  <script type="application/javascript"
>+          src="../common.js"></script>

add new line between standard js and a11y js.

>+    function doTest()
>+    {
>+      // Tests for elements rendering the original sring
>+      var IDs = ["input", "div", "editable", "textarea"];
>+  
>+
>+      // __B__r__a__v__e__ __S__i__r__ __ __R__o__b__i__n__ __ __ __r__a__n

remove one empty line

>+      //   0  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 21
>+      ////////////////////////////////////////////////////////////////////////

but please add new one between these

please put char index before char like we do in test_single.html so that index points to the char visually
;
>+      testTextAfterOffset(9, BOUNDARY_WORD_START, "Sir   ", 7, 13,
>+      testTextAfterOffset(10, BOUNDARY_WORD_START, "Sir   ", 7, 13,

should be Robin

>+      testTextAfterOffset(21, BOUNDARY_WORD_END, "", 22, 22,

please add test for 22 as well

>+      testTextBeforeOffset(21, BOUNDARY_WORD_START, "Robin   ", 11, 19,

please add test for 22 as well

>+      testTextBeforeOffset(22, BOUNDARY_WORD_END, "   ran", 16, 22,

should be "  Robin" because "... to the word end before the offset", word end before the 22 offset is "Robin" since 22 is end offset of "run".

>+      testTextAtOffset(21, BOUNDARY_WORD_START, "ran", 19, 22,

please add test for 22 as well

>+      testTextAtOffset(21, BOUNDARY_WORD_END, "   ran", 16, 22,

please add test for 22 as well

Comment 14

[Fernando Herrera]

Attachment: patch4

Updated patch

Comment 15

[alexander :surkov (:asurkov)]

Comment on attachment 491831 [details] [diff] [review]
patch4

r=me, thank you

Comment 16

[Fernando Herrera]

Attachment: patch5

Just forgot to update kOk/kTodo status for new tests.

Comment 17

[alexander :surkov (:asurkov)]

landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/b89d1824a762