Add extensive testing of interactions of text offsets line ends punctuation and whitespace

NEW
Unassigned

Status

()

4 years ago
3 years ago

People

(Reporter: tbsaunde, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Comment 1

4 years ago
Created attachment 8512909 [details] [diff] [review]
add more tests for word boundaries

This has been sitting around in one of my trees for a while, and I think its kind of worth checking in as is, but I don't really have time to improve it more atm.
Attachment #8512909 - Flags: review?(dbolter)
Comment on attachment 8512909 [details] [diff] [review]
add more tests for word boundaries

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

f=me, fine with me, Alex what say you?
Attachment #8512909 - Flags: review?(surkov.alexander)
Attachment #8512909 - Flags: review?(dbolter)
Attachment #8512909 - Flags: feedback+
Comment on attachment 8512909 [details] [diff] [review]
add more tests for word boundaries

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

::: accessible/tests/mochitest/text/test_wordboundary.html
@@ +16,5 @@
>      function doTest()
>      {
> +      var tester = getAccessible("word_test_1", nsIAccessibleText);
> +      var start = {}, end = {};
> +      tester.getTextAtOffset(0, BOUNDARY_WORD_START, start, end);

what's for?

@@ +20,5 @@
> +      tester.getTextAtOffset(0, BOUNDARY_WORD_START, start, end);
> +
> +      testTextAtOffset("pre_1", BOUNDARY_WORD_START,
> +          [[0, 5, "hello\n", 0, 6],
> +          [6, 10, "world", 6, 11]]);

nit: arguments should be lined up

@@ +47,5 @@
> +          [[0, 7, "hello.\n ", 0, 8],
> +          [8, 13, "world", 8, 13]]);
> +      testTextAtOffset("pre_10", BOUNDARY_WORD_START,
> +          [[0, 5, "hello\n", 0, 6],
> +          [6, 7, ". ", 6, 8],

I'm not sure it's separate word, it's rather todo item

@@ +51,5 @@
> +          [6, 7, ". ", 6, 8],
> +          [8, 13, "world", 8, 13]]);
> +      testTextAtOffset("pre_11", BOUNDARY_WORD_START,
> +          [[0, 5, "hello ", 0, 6],
> +          [6, 7, "hello .\n", 0, 8],

there's something wrong here

@@ +67,5 @@
> +          [[0, 7, "hello.\n ", 0, 8],
> +          [8, 13, "world", 8, 13]]);
> +      testTextAtOffset("pre_16", BOUNDARY_WORD_START,
> +          [[0, 5, "hello\n", 0, 6],
> +          [6, 7, ". ", 6, 8],

same as above

@@ +72,5 @@
> +          [8, 13, "world", 8, 13]]);
> +      testTextAtOffset("pre_17", BOUNDARY_WORD_START,
> +          [[0, 5, "hello ", 0, 6],
> +          [6, 7, "hello .\n", 0, 8],
> +          [8, 13, "world", 8, 13]]);

same as above

@@ +107,5 @@
> +      testTextAtOffset("div_9", BOUNDARY_WORD_START,
> +          [[0, 5, "hello ", 0, 6],
> +          [6, 7, kEmbedChar, 6, 7]]);
> +      testTextAtOffset("div_10", BOUNDARY_WORD_START,
> +          [[0, 6, "hello" + kEmbedChar, 0, 6]]);

how does happen it so different in expectations from div_3?

@@ +109,5 @@
> +          [6, 7, kEmbedChar, 6, 7]]);
> +      testTextAtOffset("div_10", BOUNDARY_WORD_START,
> +          [[0, 6, "hello" + kEmbedChar, 0, 6]]);
> +      testTextAtOffset("div_10_1", BOUNDARY_WORD_START,
> +          [[0, 0, " ", 0, 1],

the spec is tricky but I would say that empty string is more reasonable since " " is not a word and thus it's the case of "offset is outside the word" (https://developer.gnome.org/atk/stable/AtkText.html#atk-text-get-text-at-offset)

@@ +130,5 @@
> +      testTextAtOffset("div_16", BOUNDARY_WORD_START,
> +          [[0, 4, "hello.", 0, 6],
> +          [5, 7, kEmbedChar, 6, 7]]);
> +      testTextAtOffset("div_17", BOUNDARY_WORD_START,
> +          [[0, 6, kEmbedChar, 5, 6]]);

I'm confused since div_17 contains "hello*" but "hello" is not expected by test
Attachment #8512909 - Flags: review?(surkov.alexander)
You need to log in before you can comment on or make changes to this bug.