add getText* at caret offset mochitest

RESOLVED FIXED in mozilla22

Status

()

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

People

(Reporter: surkov, Assigned: surkov)

Tracking

unspecified
mozilla22
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
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)
Assignee: nobody → surkov.alexander
(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.
(Assignee)

Comment 3

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

Comment 5

5 years ago
(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

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

Comment 7

5 years ago
(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

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

Comment 9

5 years ago
Created attachment 726471 [details] [diff] [review]
patch
Attachment #726471 - Flags: review?(trev.saunders)
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>
Attachment #726471 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 11

5 years ago
(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
https://hg.mozilla.org/mozilla-central/rev/2c9c6ef19445
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.