852021 - add getText* at caret offset mochitest

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[alexander :surkov (:asurkov)]

since softline breaks means the same offset is shared by end line and start line then I think it makes use logical offsets in getText* implementation, i.e.

if we are at the end of line then getTextAt for line_start returns this line (not the next line to the line we are at)

Comment 2

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #1)
> since softline breaks means the same offset is shared by end line and start
> line then I think it makes use logical offsets in getText* implementation,

so, there's no '\n' in the case of soft line break?

btw should we add some tests to test_multiline.html that cover soft line breaks or do they exist some place else?

> i.e.
> 
> if we are at the end of line then getTextAt for line_start returns this line
> (not the next line to the line we are at)

that seems reasonable, though maybe we need to check that consumers are ok with it.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #2)
> (In reply to alexander :surkov from comment #1)
> > since softline breaks means the same offset is shared by end line and start
> > line then I think it makes use logical offsets in getText* implementation,
> 
> so, there's no '\n' in the case of soft line break?

correct

> btw should we add some tests to test_multiline.html that cover soft line
> breaks or do they exist some place else?

it doesn't make much sense because you will never can say where you are (at the line start of line end) and that's why caret offset trick was introduced actually and that's why I add a tests for that.

> > i.e.
> > 
> > if we are at the end of line then getTextAt for line_start returns this line
> > (not the next line to the line we are at)
> 
> that seems reasonable, though maybe we need to check that consumers are ok
> with it.

I'm scary to thing about what consumers think because they had to deal with our bugs for years. I won't be surprised if we break them by following the spec.

Comment 4

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #3)
> (In reply to Trevor Saunders (:tbsaunde) from comment #2)
> > (In reply to alexander :surkov from comment #1)
> > > since softline breaks means the same offset is shared by end line and start
> > > line then I think it makes use logical offsets in getText* implementation,
> > 
> > so, there's no '\n' in the case of soft line break?
> 
> correct

maybe its simplest to just add one in?

> > btw should we add some tests to test_multiline.html that cover soft line
> > breaks or do they exist some place else?
> 
> it doesn't make much sense because you will never can say where you are (at
> the line start of line end) and that's why caret offset trick was introduced

I'm not sure what you mean, can't you check that the last char on a line before the soft wrap is the boundary?

> > > i.e.
> > > 
> > > if we are at the end of line then getTextAt for line_start returns this line
> > > (not the next line to the line we are at)
> > 
> > that seems reasonable, though maybe we need to check that consumers are ok
> > with it.
> 
> I'm scary to thing about what consumers think because they had to deal with
> our bugs for years. I won't be surprised if we break them by following the
> spec.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #4)

> > > so, there's no '\n' in the case of soft line break?
> > 
> > correct
> 
> maybe its simplest to just add one in?

that was an idea (not sure but maybe we had a wip on it). But I'm still not sure if it's worth to do. Anyway softline breaks is somewhere in the future on the roadmap

> > > btw should we add some tests to test_multiline.html that cover soft line
> > > breaks or do they exist some place else?
> > 
> > it doesn't make much sense because you will never can say where you are (at
> > the line start of line end) and that's why caret offset trick was introduced
> 
> I'm not sure what you mean, can't you check that the last char on a line
> before the soft wrap is the boundary?

of course some tests can be added but it doesn't make a huge sense to add them since line navigation through soft line breaks is broken in general.

Comment 6

[Marco Zehe (:MarcoZ)]

(In reply to alexander :surkov from comment #3)
> I'm scary to thing about what consumers think because they had to deal with
> our bugs for years. I won't be surprised if we break them by following the
> spec.

Oh no, please fix it! :) This has been a reoccurring question why NVDA speaks the first character of the next line while the cursor is actually on the soft line break of the previous.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Marco Zehe (:MarcoZ) from comment #6)

> Oh no, please fix it! :) This has been a reoccurring question why NVDA
> speaks the first character of the next line while the cursor is actually on
> the soft line break of the previous.

it seems different bug, btw, what character should it say?

Comment 8

[Marco Zehe (:MarcoZ)]

It should say "newline" or something similar, up to the choice of the screen reader, but definitely not say the first character of the next line. I documented this in bug 473206 IIRC.

Comment 9

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 10

[Trevor Saunders (:tbsaunde)]

Comment on attachment 726471 [details] [diff] [review]
patch

>+        testTextAfterOffset(kCaretOffset, BOUNDARY_LINE_START, "two ", 6, 10,
>+                            "textarea", kTodo, kTodo, kTodo);
>+
>+        testTextAfterOffset(kCaretOffset, BOUNDARY_LINE_END, "aword", 0, 5,
>+                            "textarea", kTodo, kOk, kTodo);

so, the start of the text is a line end but not a start or what?

r=me with that explained

>+
>+        testTextAtOffset(kCaretOffset, BOUNDARY_LINE_START, "aword\n", 0, 6,
>+                         "textarea", kTodo, kOk, kTodo);
>+
>+        testTextAtOffset(kCaretOffset, BOUNDARY_LINE_END, "", 0, 0,
>+                         "textarea", kTodo, kOk, kTodo);
>+
>+        testTextBeforeOffset(kCaretOffset, BOUNDARY_LINE_START, "", 0, 0,
>+                             "textarea", kOk, kOk, kOk);
>+
>+        testTextBeforeOffset(kCaretOffset, BOUNDARY_LINE_END, "", 0, 0,
>+                             "textarea", kOk, kOk, kOk);
>+      }
>+
>+      this.getID = function moveToFirstLineStart_getID()
>+      {
>+        return "move to first line start";
>+      }
>+    }
>+
>+    function moveToFirstLineEnd()
>+    {
>+      this.__proto__ = new moveToLineEnd("textarea", 5);
>+
>+      this.finalCheck = function moveToFirstLineStart_finalCheck()
>+      {
>+        testTextAfterOffset(kCaretOffset, BOUNDARY_LINE_START, "two ", 6, 10,
>+                            "textarea", kTodo, kTodo, kTodo);
>+
>+        testTextAfterOffset(kCaretOffset, BOUNDARY_LINE_END, "\ntwo ", 5, 10,
>+                            "textarea", kTodo, kTodo, kTodo);
>+
>+        testTextAtOffset(kCaretOffset, BOUNDARY_LINE_START, "aword\n", 0, 6,
>+                         "textarea", kTodo, kOk, kTodo);
>+
>+        testTextAtOffset(kCaretOffset, BOUNDARY_LINE_END, "aword", 0, 5,
>+                         "textarea", kOk, kOk, kOk);
>+
>+        testTextBeforeOffset(kCaretOffset, BOUNDARY_LINE_START, "", 0, 0,
>+                             "textarea", kTodo, kOk, kTodo);
>+
>+        testTextBeforeOffset(kCaretOffset, BOUNDARY_LINE_END, "", 0, 0,
>+                             "textarea", kTodo, kOk, kTodo);
>+      }
>+
>+      this.getID = function moveToFirstLineEnd_getID()
>+      {
>+        return "move to first line end";
>+      }
>+    }
>+
>+    var gQueue = null;
>+    function doTest()
>+    {
>+      SimpleTest.expectAssertions(5);
>+
>+      gQueue = new eventQueue();
>+      gQueue.push(new moveToLastLineEnd());
>+      gQueue.push(new moveToLastLineStart());
>+      gQueue.push(new moveToMiddleLineStart());
>+      gQueue.push(new moveToMiddleLineEnd());
>+      gQueue.push(new moveToFirstLineStart());
>+      gQueue.push(new moveToFirstLineEnd());
>+      gQueue.invoke(); // will call SimpleTest.finish();
>+    }
>+
>+    SimpleTest.waitForExplicitFinish();
>+    addA11yLoadEvent(doTest);
>+  </script>
>+</head>
>+<body>
>+
>+  <a target="_blank"
>+     title="nsIAccessibleText getText related functions tests at caret offset"
>+     href="https://bugzilla.mozilla.org/show_bug.cgi?id=852021">
>+   Bug 852021
>+  </a>
>+  <p id="display"></p>
>+  <div id="content" style="display: none"></div>
>+  <pre id="test">
>+
>+  <textarea id="textarea" cols="5">aword
>+two words</textarea>
>+  </pre>
>+</body>
>+</html>

Comment 11

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #10)
> Comment on attachment 726471 [details] [diff] [review]
> patch
> 
> >+        testTextAfterOffset(kCaretOffset, BOUNDARY_LINE_START, "two ", 6, 10,
> >+                            "textarea", kTodo, kTodo, kTodo);
> >+
> >+        testTextAfterOffset(kCaretOffset, BOUNDARY_LINE_END, "aword", 0, 5,
> >+                            "textarea", kTodo, kOk, kTodo);
> 
> so, the start of the text is a line end but not a start or what?

we are at 0 offset, so first one (line start) is "after to after", i.e. from line start after the offset to line start after that line start after the offset, it's a second line "two " (it would contain '\n' but this is soft line break.

the second one (line end) is "after or at to after", 0 and text length offsets are both start and end lines, thus we get the first line "aword".

hope it's clear enough

Comment 12

[alexander :surkov (:asurkov)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/2c9c6ef19445

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2c9c6ef19445