298690 - Caret skips blank line using down-arrow key in preformatted text (e.g. View Source)

Status: RESOLVED FIXED

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

Description

[Hideo Oshima]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050624 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050624 Firefox/1.0+

In the view source window, caret skips blank line by down arrow key.


Reproducible: Always

Steps to Reproduce:
1.Open http://www.mozilla.org/
2.Open source window
3.Click most top left position and display caret
4. press down arrow key twice

Actual Results:  
caret moves to the 4th line.

Expected Results:  
caret moves to the 3rd. line.
don't skip the blank line.

Moving caret by left or right arrow key doesn't skip the blank line.

Comment 1

[Uri Bernstein]

Seeng this on a Mac => All/All. Can't find a duplicate, so confirming.

This can also be reproduced in the browser itslef (i.e. not he source view), in
Caret Browsing mode, inside a <pre> element (I'll attach a testcase).

Changing component to Core/Keyboard: Navigation

Comment 2

[Uri Bernstein]

Attachment: testcase

In Caret Browsing mode, use the up/down arrow keys to navigate. The second
(blank) line is skipped.

Comment 3

[Uri Bernstein]

The bug is not specific to "View Source", so changing summary accordingly.

Comment 4

[Uri Bernstein]

This is a regression from bug 192320. See bug 192320 comment #15 which
anticipated this bug. This doesn't actually happen in composer only because
BRFrames in that case are assigned a width of 1 (as a temporary workaround for
some other bug).

Comment 5

[Uri Bernstein]

Comment #4 isn't accurate - this was broken even before bug 192320 was fixed,
due to other issues. I'll attach a patch in which I'll explain more.

Comment 6

[Uri Bernstein]

Attachment: patch v1

This patch contains three changes to places where zero-width frames are
special-cased. All three are required to fix this bug.

1. In nsLineIterator::FindFrameAt(). This is what comment #4 was about. Bug
192320 introduces this test for a case where the frame was actually both
zero-width and zero-height (whitespace following a <br> and followed
immediatley by a <p>). So I simply limited the test to that case (in our case
the frame is zero-width, but not zero-height).

2. The test for zero-width in nsFrame::GetNextPrevLineFromeBlockFrame() was
added as part of the fix to bug 159207, and is explained (I think) in bug
159207 comment #15. I admit I don't fully understand what situation that
comment is referring to, but I did various tests clicking around <hr>s in
composer, with the zero-width check removed, and I didn't see any regressions.

3. In nsTextFrame::GetPosition() I couldn't find the reason for returning
NS_ERROR_FAILURE for zero-width text. I suspect it's just because the
binary-search algorithm fails for that case. So what I did is to simply return
the frame content offset (bypassing the binary search).

I'm a bit nervous about #2 and #3, but I believe that zero-width frames (which
might occur normally, as in this this case) should not generally lead to
failure of methods when they can be treated as "regular" frames.

Comment 7

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

Comment on attachment 199937 [details] [diff] [review]
patch v1

It's risky, but I agree with you.

Comment 8

[Uri Bernstein]

Attachment: same, unbroken

Not sure what happened there. Let's try this again.

Comment 9

[Simon Montagu :smontagu]

Checked in.

Comment 10

[baffclan]

reproduce with Firefox/2005103105-trunkl/WinXP.
however, it is right key not down key.

Steps to Reproduce:
1. open http://www.mozilla.org/
2. open source window
3. click Line 20 left position and display caret
4. press right arrow key eight times
5. caret moves to the 22th line.

Comment 11

[Uri Bernstein]

(In reply to comment #10)
> reproduce with Firefox/2005103105-trunkl/WinXP.
> however, it is right key not down key.

This bug is specifically about the down-arrow key. If you see other bugs (with different keys), please submit them separately.

Comment 12

[baffclan]

(In reply to comment #11)
> This bug is specifically about the down-arrow key. If you see other bugs (with
> different keys), please submit them separately.
thanks for comment.
I reported it to bug 314519.

Comment 13

[Uri Bernstein]

Checked in to 1.8.1 branch:
Checking in mozilla/layout/generic/nsLineBox.cpp;
/cvsroot/mozilla/layout/generic/nsLineBox.cpp,v  <--  nsLineBox.cpp
new revision: 1.84.4.1; previous revision: 1.84
done
Checking in mozilla/layout/generic/nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.574.2.8; previous revision: 3.574.2.7
done
Checking in mozilla/layout/generic/nsTextFrame.cpp;
/cvsroot/mozilla/layout/generic/nsTextFrame.cpp,v  <--  nsTextFrame.cpp
new revision: 1.513.4.10; previous revision: 1.513.4.9
done