mochitest with extra whitespaces for nsIAccessibleText

RESOLVED FIXED

Status

()

Core
Disability Access APIs
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Fernando Herrera, Assigned: Fernando Herrera)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

8 years ago
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
(Assignee)

Comment 1

8 years ago
Created attachment 489075 [details] [diff] [review]
patch1

Initial bunch of tests. Still need to add tests for divs where the string is collapsed
Attachment #489075 - Flags: review?(surkov.alexander)
Assignee: nobody → fherrera
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Blocks: 452769
(In reply to comment #1)
> Initial bunch of tests. Still need to add tests for divs where the string is
> collapsed

use pre tag
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?
(Assignee)

Comment 4

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

Comment 5

8 years ago
Created attachment 490062 [details] [diff] [review]
patch2

Updated patch extending test to div and editable elements inside a <pre>
Attachment #489075 - Attachment is obsolete: true
Attachment #489075 - Flags: review?(surkov.alexander)
(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?
Btw, does ATK spec have definition of 'word' term? I wonder about 'away...' where's the word here.
and btw why do you need to test '...' boundaries here? In general I would like to see small test string as possible.
(Assignee)

Comment 9

8 years ago
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
(Assignee)

Comment 10

8 years ago
Should we also skip line boundaries testing here or is there any reason to keep them?
(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.
(Assignee)

Comment 12

8 years ago
Created attachment 491599 [details] [diff] [review]
patch3

Updated patch with new test string
Attachment #490062 - Attachment is obsolete: true
Attachment #491599 - Flags: review?(surkov.alexander)
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
Attachment #491599 - Flags: review?(surkov.alexander)
(Assignee)

Comment 14

8 years ago
Created attachment 491831 [details] [diff] [review]
patch4

Updated patch
Attachment #491599 - Attachment is obsolete: true
Attachment #491831 - Flags: review?(surkov.alexander)
Comment on attachment 491831 [details] [diff] [review]
patch4

r=me, thank you
Attachment #491831 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 16

8 years ago
Created attachment 491833 [details] [diff] [review]
patch5

Just forgot to update kOk/kTodo status for new tests.
Attachment #491831 - Attachment is obsolete: true
landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/b89d1824a762
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.