320357 - Magnifier can't work with wrapped text in full screen magnification.

Status: RESOLVED FIXED

(Firefox :: Disability Access, defect)

Description

[Evan Yan]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051214 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051214 Firefox/1.6a1

When there is wrapped text, like input text more than one line in a text area, and a block of text in a web page with out a new line tag, magnifier can not focus following the caret. Instead magnifier treat the wrapped text as in one line and count wrong coords.

Reproducible: Always

Steps to Reproduce:
1. Open magnifier.
2. Input some text more than one line in the "additional comments" text area in this page.
3. When the text reach the end of a line and be wrapped, magnifier does not bring the focus back correctly.

Actual Results:  
magnifier loses focus

Expected Results:  
magnifier brings focus following the caret.

Comment 1

[Ginn Chen]

In accessible/src/atk/nsAccessibleText.cpp, it doesn't consider wraps

751   //add the width of the string before current char
752   nsAutoString beforeString;
753   nscoord beforeWidth;
754   if (NS_SUCCEEDED(GetText(0, aOffset, beforeString)) &&
755       NS_SUCCEEDED(rc->GetWidth(beforeString, beforeWidth))) {
756     frameScreenRect.x += NSTwipsToIntPixels(beforeWidth, t2p);
757   }

Comment 2

[Ginn Chen]

Also in accessible/src/atk/nsAccessibleText.cpp, it doesn't consider wraps

711   nsIFrame *frame = shell->GetPrimaryFrameFor(content);
712   NS_ENSURE_TRUE(frame, NS_ERROR_FAILURE);
713 
714   nsIntRect frameScreenRect = frame->GetScreenRectExternal();

Comment 3

[Ginn Chen]

Attachment: patch

Use another method to get x,y point.
It works for me.

BTW:With this patch, we still have a small issue with magnifier. When you press "End" in a static paragraph, caret blinks at the end of line, but magnifier focused the next line. It relates to the offset value of text-caret-moved event. It's not a bug of GetCharacterExtents(), should be fixed separately.

Comment 4

[Ginn Chen]

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

not work with wrap text without BR in text area

Comment 5

[Ginn Chen]

Attachment: patch v2

patch v2
fixed softbreak issue for both static text and editable text

Comment 6

[Jason Grieves]

(In reply to comment #5)
> Created an attachment (id=206385) [edit]
> patch v2
> 
> patch v2
> fixed softbreak issue for both static text and editable text
> 
Error during build

 In member function ‘virtual nsresult nsAccessibleText::GetCharacterExtents(PRInt32, PRInt32*, PRInt32*, PRInt32*, PRInt32*, nsAccessibleCoordType)’:
/home/jgrieves/downloads/mozilla/accessible/src/atk/nsAccessibleText.cpp:772: error: no matching function for call to ‘nsIPresShell::GetPrimaryFrameFor(nsCOMPtr<nsITextContent>&)’
../../../dist/include/layout/nsIPresShell.h:306: note: candidates are: virtual nsresult nsIPresShell::GetPrimaryFrameFor(nsIContent*, nsIFrame**) const
make[5]: *** [nsAccessibleText.o] Error 1

relevant lines..

    771 NS_ENSURE_TRUE(textContent, NS_ERROR_FAILURE);
    772 textFrame = shell->GetPrimaryFrameFor(textContent);
    773 NS_ENSURE_TRUE(textFrame, NS_ERROR_FAILURE);

Comment 7

[Ginn Chen]

(In reply to comment #6)

> Error during build
> 

This patch is for trunk not MOZILLA_1_8_BRANCH (Firefox 1.5/2.0)
GetPrimaryFrameFor changed on trunk, see Bug 303779.

Comment 8

[Aaron Leventhal]

Comment on attachment 206385 [details] [diff] [review]
patch v2

bug 320357 - magnifier and wrapped text
Why doesn't OffsetToDomPoint take an nsIEditor instead of nsISupports? That's extra work to convert it back and forth.

Does this code exist somewhere else in the Mozilla tree? If it does we should have more code reuuse.

Comment 9

[Ginn Chen]

Attachment: patch v3, change nsISupports to nsIEditor for OffsetToDomPoint

BTW: I didn't find these code exsited other place in Mozilla.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Blake, do you know if these guys can use caret-related code here?

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Is this magnifier following the caret around? If it is, then you might be able to use nsICaret::GetCaretCoordinates, which will return the rect that the caret occupies. I'm not exactly sure what this magnifier does...

Comment 12

[Ginn Chen]

Magnifier cooperates with mozilla through ATK 2.0 API, it can't do GetCaretCoordinates directly.
It uses atk_text_get_caret_offset and atk_text_get_character_extents.

Comment 13

[Ginn Chen]

roc, do you more information?

Comment 14

[Aaron Leventhal]

Make sure new nsHyperTextAccessible supports this -- I believe after bug 312093 it will.

Comment 15

[Ginn Chen]

I can't verify this bug now.
Magnifier doesn't work on my box. :(
And I met crashes with multiline textfield (ROLE_ENTRY) when orca is running.
Crashed at
nsHyperTextAccessible::GetRelativeOffset(nsIPresShell *aPresShell, nsIFrame *aFromFrame, PRInt32 aFromOffset, nsSelectionAmount aAmount, nsDirection aDirection, PRBool aNeedsStart)

aFromFrame is null.

Comment 16

[Aaron Leventhal]

Attachment: Fixes endOffset logic in GetPosAndText(), clarify with some more comments

Ginn, I don't know if this means the bug is fixed, but it takes care of the GetPosAndText() problem you emailed me about.

Comment 17

[Ginn Chen]

(In reply to comment #16)
> Ginn, I don't know if this means the bug is fixed, but it takes care of the
> GetPosAndText() problem you emailed me about.
> 

right, I got a similar fix yesterday.
I think it can't solve this bug, but may solve part of bug 345825.

Comment 18

[Aaron Leventhal]

Did you want me to post this patch to bug 345825?

Comment 19

[Ginn Chen]

Comment on attachment 230599 [details] [diff] [review]
Fixes endOffset logic in GetPosAndText(), clarify with some more comments

moved to bug 345825, thank you Aaron.

Comment 20

[Aaron Leventhal]

Attachment: Provides correct results when used with fix from bug 345825

Comment 21

[Aaron Leventhal]

Attachment: Also will provide bounds if the character was an embedded object character

Comment 22

[Tim Miao]

This bug can be reproduced with full screen magnification.

Comment 23

[Aaron Leventhal]

Tim, and we discussed via email -- the bug cannot be reproduced with partial magnification, correct?

Comment 24

[Evan Yan]

There is a regression that the magnifier won't follow inputing or caret browsing.
this regression blocks this bug.
I filed bug 349860 for it.

Comment 25

[Aaron Leventhal]

Tim, please verify when you have a chance.