Closed Bug 345825 Opened 19 years ago Closed 19 years ago

getTextAtOffset doesn't work correctly

Categories

(Firefox :: Disability Access, defect)

Product:
Firefox
Component:
Disability Access
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: ginnchen+exoracle, Assigned: aaronlev)

References

Details

(Keywords: access)

Attachments

(5 files, 8 obsolete files)

python script
17.16 KB, text/x-python
Details
Test case 1
48 bytes, text/html
Details
Test case 2
100 bytes, text/html
Details
merged with aaron's patch in bug 320357
7.91 KB, patch
aaronlev
: review+
Details | Diff | Splinter Review
Correct patch
26.06 KB, patch
ginnchen+exoracle
: review+
Details | Diff | Splinter Review
1. Run the attached standalone python application in an xterm 2. Run Firefox open the test case html file and click on its window. Press F11. The python script was copied from bug 317482. I modified a bit to make sure it won't be an infinite loop due to the bug of getTextAtOffset.
Attached file python script —
Attached file Test case 1 —
Attached file Test case 2 —
Actual result of test case 1: Text (len=17): abcd efgh 123456 line (start=0, end=0, x=925, y=416, width=7, height=20): getTextAtOffset(0, Accessibility.TEXT_BOUNDARY_LINE_START) return 0 for both start and end I debugged into it and found PeekOffset to end of line (abcd<br>), return the parent node with offset 1. The parent node is <body>, we didn't deal with this case in nsHyperTextAccessible::DOMPointToOffset.
Status: NEW → ASSIGNED
Actual result of test case 2: Text (len=23): abcdef ghijkl &#65532; 123456 line (start=0, end=7, x=925, y=385, width=57, height=15): abcdef &#65532; 12345 line (start=0, end=0, x=925, y=385, width=7, height=15): line (start=6, end=7, x=967, y=385, width=15, height=15): &#65532; line (start=15, end=0, x=1023, y=400, width=15, height=15): Line 1's offset is almost right, but the string is wrong. Line 2 and Line 3's start offset is wrong.
Now test case 2 works fine. Also fixes some lines to fit 80 width.
Attachment #230703 - Flags: review?(aaronleventhal)
Attachment #230703 - Flags: review?(aaronleventhal) → review+
Checking in nsHyperTextAccessible.cpp; /cvsroot/mozilla/accessible/src/html/nsHyperTextAccessible.cpp,v <-- nsHyperTextAccessible.cpp new revision: 1.10; previous revision: 1.9 don
Ginn says: > We still need a patch for test case 1. > Sometimes, PeekOffset returns the parent frame.
Assignee: ginn.chen → aaronleventhal
Attachment #231438 - Flags: review?(ginn.chen)
Comment on attachment 231438 [details] [diff] [review] Fixes test case 1 and other problems It doesn't fix test case 1, and it breaks test case 2 again.
Attachment #231438 - Flags: review?(ginn.chen) → review-
Attached patch Fixed again for test case 1 & 2 (obsolete) — — Splinter Review
Ginn, I used Javascript versions of those testcases for now. Let me know how they work with your Python testcase.
Attachment #231803 - Flags: review?(ginn.chen)
Comment on attachment 231803 [details] [diff] [review] Fixed again for test case 1 & 2 Yes, test case 2 works fine, but test case 1 still no luck. In short, this patch didn't change the result.
Attachment #231803 - Attachment is obsolete: true
Attachment #231988 - Flags: review?(ginn.chen)
Attachment #231803 - Flags: review?(ginn.chen)
Attachment #231988 - Attachment is obsolete: true
Attachment #231988 - Flags: review?(ginn.chen)
Ginn, I'm still testing with Javascript but I more carefully duplicated the test cases and did indeed find problems to fix.
Attachment #231990 - Attachment is obsolete: true
Attachment #231990 - Flags: review?(ginn.chen)
(In reply to comment #16) > Created an attachment (id=232070) [edit] > Should compile on Linux, but haven't looked at Ginn's results for testcase 1 > yet > +// ATK 1.3.0 or later +#if ATK_MAJOR_VERSION >=2 || \ + (ATK_MAJOR_VERSION == 1 && ATK_MINOR_VERSION >= 3) +#ifdef USE_ATK_GET_RANGE_EXTENTS ******should be #define USE_ATK_GET_RANGE_EXTENTS +#endif + rect->x = extX; + rect->y = extY; + rect->width = extWidth; + rect->height = extHeight; ****** should be aRect->
Attached patch WIP (obsolete) — — Splinter Review
Ginn's results for current patch, posted from IRC: <ginn> aOffset = 0, *startOffset =0, *endOffset =1 <ginn> aOffset = 1,2,3,4 same *startOffset *endOffset <ginn> aOffset =5,6,7,8,9 fails <ginn> aOffset = 10, *startOffset =10, *endOffst =17 <aaronlev> for beginline boundary type? <ginn> TEXT_BOUNDARY_LINE_START <ginn> getTextAtOffset
Attached patch WIP (obsolete) — — Splinter Review
Attachment #232719 - Attachment is obsolete: true
Attached patch Fixes testcase 1 (obsolete) — — Splinter Review
Attachment #231438 - Attachment is obsolete: true
Attachment #232070 - Attachment is obsolete: true
Attachment #233122 - Attachment is obsolete: true
Attached patch Correct patch — — Splinter Review
Ginn, this time I built and tested with your Python script. The difference was that your testcase was right underneath the <body> -- the nsDocAccessible was the hypertext in that case.
Attachment #233838 - Attachment is obsolete: true
Attachment #233842 - Flags: review?(ginn.chen)
Comment on attachment 233842 [details] [diff] [review] Correct patch r=me Some comments: 1) + aIface->get_range_extents = getRangeExtentsCB; and +void +getRangeExtentsCB(AtkText *aText, gint aStartOffset, gint aEndOffset, + AtkCoordType aCoords, AtkTextRectangle *aRect) +{ (nsMaiInterfaceText.cpp) should be quoted by USE_ATK_GET_RANGE_EXTENTS 2) It might be good to have a null check for aRect in getRangeExtentsCB
Attachment #233842 - Flags: review?(ginn.chen) → review+
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: