240665 - Can't navigate through some page lines using left or right arrow

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Ginn Chen]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4.1) Gecko/20040117
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4.1) Gecko/20040117

Can't go through blank block e.g.<b><i></i></b> while caret browsing.
Reproduced on Mozilla 1.7b

Reproducible: Always
Steps to Reproduce:
1.Press F7 to turn on Caret browsing
2.Open the attached test page.
3.Put the caret on the line.
3.Press right arrow or left arrow, you can't go to "EFG" from "ABCD" or go to
"ABCD" from "EFG"

Actual Results:  
can't go to "EFG" from "ABCD" or go to "ABCD" from "EFG"

Expected Results:  
can go through

Comment 1

[Ginn Chen]

Attachment: testcase

Comment 2

[Ginn Chen]

Attachment: patch

Comment 3

[Aaron Leventhal]

Ginn, this looks like a high quality fix, but I'm not totally sure how it works
(without launching my debugger and stepping through line by line).

Ginn, can you add some comments to the code which explains it a little better?
Also, if you understand the other parts of this method, adding comments to their
code will help future developers. I know you didn't write it, but this is
obviously an important area of the code that few people understand. So, maybe we
can improve the clarity of the code as we go? :)

Comment 4

[Ginn Chen]

I'm working on this method, it's a long long method, about 300 lines, and
nsTextFrame::PeekOffset is a 500 lines long method. I'm still trying to
understand it.
I find a lot of problems on keyboard navigation, the behavior differs from
Mozilla 1.4 to 1.7, but both are not correct enough. Especially for Form,
TextBox, Align, Div. I'm trying to make it work better.

For this patch, at the beginning of the method, we set result equal to FAILURE,
3596   nsresult result = NS_ERROR_FAILURE;
And if we are peeking offset in the frame, we have not set the result to OK.
3630           else
3631           {
3632             if (aPos->mDirection == eDirNext)
3633               aPos->mContentOffset = newOffset +1;
3634             else
3635               aPos->mContentOffset = newOffset;//to beginning of frame
3636           }
After the break, it will return NS_ERROR_FAILURE to the caller.
3639       break;
3640     }//drop into no amount

It prevents entering the frame. We will try to enter it and get failed for each
keystroke.
If the block get some text, nsTextFrame::PeekOffset deal with it without calling
 nsFrame::PeekOffset.

Comment 5

[Aaron Leventhal]

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

Thank you Ginn. r=aaronl

Comment 6

[Henry Jia]

sr=henry

Comment 7

[Kyle Yuan]

patch checked in.