Arabic and Hebrew characters bounds are incorrect in a11y APIs

RESOLVED FIXED in mozilla27

Status

()

Core
Disability Access APIs
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: Aaron Leventhal, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla27
x86
All
access
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [bk1])

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
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.
(Reporter)

Comment 1

9 years ago
Eitan, I'm not sure about the Hebrew part -- can you test via the text interface in ATK/AT-SPI?
(Assignee)

Comment 2

9 years ago
finally we got request to support Arabic languages. Are there screen readers for them, right?

Comment 3

9 years ago
Yes, JAWS does support Arabic, and Dolphin's HAL/Supernova does it, too. This one came directly from Dolphin.
This bug needs re-confirmation. If it still exists it seems like a major issue to me. Marco can you confirm?
Whiteboard: [bk1]
I get an NS_ERROR_FAILURE when trying to query the extents.
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.
(Assignee)

Comment 7

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

Updated

4 years ago
Blocks: 887794
(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.
(Assignee)

Comment 9

4 years ago
(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.
just set the left to be the minimum of GetPointFromOffset(k) and GetPointFromOffset(k+1), and the right to the max.
just make sure you call those on the same frame, so you don't get confused at points where the text direction changes.
(Assignee)

Comment 12

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

Comment 14

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

Comment 15

4 years ago
(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
Flags: in-testsuite+
Depends on: 921658
https://hg.mozilla.org/mozilla-central/rev/616819179a67
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.