Last Comment Bug 466481 - Arabic and Hebrew characters bounds are incorrect in a11y APIs
: Arabic and Hebrew characters bounds are incorrect in a11y APIs
Status: RESOLVED FIXED
[bk1]
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: x86 All
-- normal with 1 vote (vote)
: mozilla27
Assigned To: alexander :surkov
:
: alexander :surkov
Mentors:
Depends on: 921658
Blocks: texta11y 2013q3a11y
  Show dependency treegraph
 
Reported: 2008-11-24 08:14 PST by Aaron Leventhal
Modified: 2013-09-27 19:21 PDT (History)
8 users (show)
surkov.alexander: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (4.87 KB, patch)
2013-09-26 11:55 PDT, alexander :surkov
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review

Description User image Aaron Leventhal 2008-11-24 08:14:40 PST
Methods AccessibleText::getCharacterExtents and ISimpleDOMText::get_unclipppedSubstringBounds are returning all zeros for Arabic characters. This probably affects Hebrew as well.

Example page:
http://nattiq.com/ar

Reported by David Carrington from HAL/Lunar/Supernova team.
Comment 1 User image Aaron Leventhal 2008-11-24 11:56:42 PST
Eitan, I'm not sure about the Hebrew part -- can you test via the text interface in ATK/AT-SPI?
Comment 2 User image alexander :surkov 2008-11-24 19:26:03 PST
finally we got request to support Arabic languages. Are there screen readers for them, right?
Comment 3 User image Marco Zehe (:MarcoZ) 2008-11-24 23:03:52 PST
Yes, JAWS does support Arabic, and Dolphin's HAL/Supernova does it, too. This one came directly from Dolphin.
Comment 4 User image David Bolter [:davidb] 2011-11-01 14:33:44 PDT
This bug needs re-confirmation. If it still exists it seems like a major issue to me. Marco can you confirm?
Comment 5 User image Eitan Isaacson [:eeejay] 2011-11-01 15:58:02 PDT
I get an NS_ERROR_FAILURE when trying to query the extents.
Comment 6 User image Günter Hillenbrand 2013-07-17 07:56:44 PDT
Cobra from BAUM Retec does also support Arabic and Hebrew and I also have the problem, that IAccessibleText::get_characterExtents always returns E_FAIL on Arabic and Hebrew characters. Tested with current Firefox Release 22.
Comment 7 User image alexander :surkov 2013-09-24 11:01:30 PDT
nsIFrame::GetPointFromOffset(0) returns x coordinate smaller than x coordinate of GetPointFromOffset(1), so we get negative width box what is considered empty (http://mxr.mozilla.org/mozilla-central/source/accessible/src/generic/HyperTextAccessible.cpp#199).

The question is how offsets counting and coordinates are changed for rtl languages?
Comment 8 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2013-09-24 21:12:52 PDT
(In reply to alexander :surkov from comment #7)
> nsIFrame::GetPointFromOffset(0) returns x coordinate smaller than x
> coordinate of GetPointFromOffset(1)

Are you sure? that doesn't sound right to me. See nsTextFrame::GetPointFromOffset.
Comment 9 User image alexander :surkov 2013-09-25 06:02:15 PDT
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> (In reply to alexander :surkov from comment #7)
> > nsIFrame::GetPointFromOffset(0) returns x coordinate smaller than x
> > coordinate of GetPointFromOffset(1)
> 
> Are you sure? that doesn't sound right to me. See
> nsTextFrame::GetPointFromOffset.

sorry, vise versa. 

How can I find x1 (left) and x2 (right) coordinates of the char? Because the current logic of getting points from offsets (k, k+1) doesn't work for rtl languages.
Comment 10 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2013-09-25 19:21:48 PDT
just set the left to be the minimum of GetPointFromOffset(k) and GetPointFromOffset(k+1), and the right to the max.
Comment 11 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2013-09-25 19:23:41 PDT
just make sure you call those on the same frame, so you don't get confused at points where the text direction changes.
Comment 12 User image alexander :surkov 2013-09-26 11:55:41 PDT
Created attachment 810706 [details] [diff] [review]
patch
Comment 13 User image Trevor Saunders (:tbsaunde) 2013-09-26 14:27:21 PDT
Comment on attachment 810706 [details] [diff] [review]
patch

>+    if (frameTextStartPoint.x < frameTextEndPoint.x) {
>+      frameScreenRect.x += frameTextStartPoint.x;
>+      frameScreenRect.width = frameTextEndPoint.x - frameTextStartPoint.x;
>+    } else {
>+      frameScreenRect.x += frameTextEndPoint.x;
>+      frameScreenRect.width = frameTextStartPoint.x - frameTextEndPoint.x;

I think
frameScreenRect.x += std::min(frameTextStartPoint.x, frameTextEndPoint.x);
frameScreenRect.width = Abs(frameTextStartPoint.x - frameTextEndPoint.x);
would be a little clearer.

I'm going to assume the text in the test is RTL but actually have no clue.
Comment 14 User image alexander :surkov 2013-09-26 16:05:10 PDT
(In reply to Trevor Saunders (:tbsaunde) from comment #13)

> I think
> frameScreenRect.x += std::min(frameTextStartPoint.x, frameTextEndPoint.x);
> frameScreenRect.width = Abs(frameTextStartPoint.x - frameTextEndPoint.x);
> would be a little clearer.

ok

> I'm going to assume the text in the test is RTL but actually have no clue.

I copied it from site from description
Comment 15 User image alexander :surkov 2013-09-27 07:18:38 PDT
(In reply to alexander :surkov from comment #14)

> > I'm going to assume the text in the test is RTL but actually have no clue.
> 
> I copied it from site from description

and what's more important the mochitest failed on it without the patch

landed, http://hg.mozilla.org/integration/mozilla-inbound/rev/616819179a67
Comment 16 User image Wes Kocher (:KWierso) 2013-09-27 19:21:37 PDT
https://hg.mozilla.org/mozilla-central/rev/616819179a67

Note You need to log in before you can comment on or make changes to this bug.