192320 - Cannot down arrow in file

Status: VERIFIED FIXED

(Core :: DOM: Selection, defect)

Description

[Aaron Leventhal]

When I'm on line 1 of this document, I cannot down arrow.

Comment 1

[Aaron Leventhal]

Attachment: Test case

Comment 2

[Joe Francis]

to selection guru ->

Comment 3

[Joe Francis]

NS7.01 from 11/20/02 does not have this bug.  My tip mozilla build from a week
ago or so show it, though.

Comment 4

[Joe Francis]

This source has a p inside a font tag.  The "block in inline" quirks code may be
involved.

Comment 5

[saari (gone)]

nominating EDITORBASE

Comment 6

[saari (gone)]

EDITORBASE+

Comment 7

[Kai Engert (:KaiE:)]

I think this looks like a nice bug to do my first steps with the editor code.
Can I take it?

Comment 8

[Kai Engert (:KaiE:)]

-> me

I stepped through the code with the debugger.

Although the display only shows 2 lines, the internal data structures counts the
content as 5 (logical) lines.

When having selected something on visible line 1, the code operates on data
structure line 0.

When doing shift-cursor-down, the code goes to data structure line 1, and calls
nsLineIterator::FindFrameAt()

This data structure line 1 has a position of 0/555, but a size of 0/0.

I believe the failure happens when doing the check for 
  aX >= line->mBounds.XMost()
is reached, which results in signaling success to the caller and that
"aXIsAfterLastFrame".

This prevents the caller code at nsFrame::GetNextPrevLineFromBlockFrame to not
test later lines and fails somehow.

I believe the fix is to test for the special case where width is zero and fail
in FindFrameAt. This will cause the caller to continue searching in the next line.

Comment 9

[Kai Engert (:KaiE:)]

Attachment: Patch v1

This patch appears to fix the problem.

Comment 10

[Kai Engert (:KaiE:)]

jfrancis: Although you said you don't see the problem with NS 7.01, I do see it
with 7.01

Comment 11

[rubydoo123]

I can repro this one using the current trunk build on win32 (2003031804)

Comment 12

[mjudge]

Comment on attachment 117721 [details] [diff] [review]
Patch v1

as long as you have tested it reasonably well

Comment 13

[Kai Engert (:KaiE:)]

Comment on attachment 117721 [details] [diff] [review]
Patch v1

Kin, can you please sr?

Comment 14

[kinmoz]

Comment on attachment 117721 [details] [diff] [review]
Patch v1

sr=[e-mail]

Comment 15

[kinmoz]

Comment on attachment 117721 [details] [diff] [review]
Patch v1

I think I may have hastily sr'd this patch ... kai please test the case where
you arrow though blank lines ... I believe blank lines (in composer) only have
brs and brs don't have any dimensions so I'm not sure if this "blank" line will
be skipped by your patch.

Also, does this code get hit from selection via mouse?

Comment 16

[Kai Engert (:KaiE:)]

It seems to work correctly.
I have tested the attached testcase on Windows and Linux.

In addition to the testcase I have used another testcase, where instead of a
single <br> the file contains <br><br><br>

With and without the patch, you have to press cursor down once for each <br>
that is present. That behaviour is not changed by the patch.

Note there is no visual feedback when selecting the empty lines.

I've also tested what happens when using the mouse to select. Again no visual
feedback for selecting the empty lines, but it still works.

Another test I did was to select all and paste it into an ascii editor. With and
without the patch the same amount of whitespace arrives at the destination.


I'm not sure whether that code change influences the mouse selection. Should I
trace whether that code is getting hit?

Comment 17

[Kai Engert (:KaiE:)]

checked in, fixed

Comment 18

[Dave Barrowman]

I have filed bug 200416, bug 200417, and bug 200418 which may be related to the
fix for this bug.

Comment 19

[Kai Engert (:KaiE:)]

I have tested all the three bugs you mentioned. I can reproduce all bugs with
earlier versions of Mozilla, the checkin to this bug did not change the
behaviour for me.

Comment 20

[Kathleen :Brade]

reopening; this is a duplicate of 192320 which Kaie just fixed

Comment 21

[Kathleen :Brade]

oops; wrong bug!

Comment 22

[Kathleen :Brade]

verifying with my mac debug build from today
carryover keywords from dupe

Comment 23

[Kathleen :Brade]

*** Bug 192856 has been marked as a duplicate of this bug. ***

Comment 24

[Kathleen :Brade]

*** Bug 194637 has been marked as a duplicate of this bug. ***

Comment 25

[Kathleen :Brade]

*** Bug 183137 has been marked as a duplicate of this bug. ***

Comment 26

[Jay Davis]

This fix was long needed and makes for a much more pleasant experience (for me
anyway).  I can now down arrow on any page....except for pages with frames.  Why
can't the main part of any page with frames be selected by default so that the
arrow keys work?  Right now, using the arrow key will do nothing at all.