Cannot down arrow in file

VERIFIED FIXED

Status

()

Core
Selection
--
major
VERIFIED FIXED
16 years ago
15 years ago

People

(Reporter: Aaron Leventhal, Assigned: kaie)

Tracking

({access, testcase, topembed+})

Trunk
access, testcase, topembed+
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: EDITORBASE+ edt_x3)

Attachments

(2 attachments)

(Reporter)

Description

16 years ago
When I'm on line 1 of this document, I cannot down arrow.
(Reporter)

Updated

16 years ago
Component: Editor: Core → Selection
(Reporter)

Comment 1

16 years ago
Created attachment 113841 [details]
Test case

Comment 2

16 years ago
to selection guru ->
Assignee: jfrancis → mjudge
QA Contact: sujay → pmac

Comment 3

16 years ago
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

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

Comment 5

16 years ago
nominating EDITORBASE
Whiteboard: EDITORBASE

Comment 6

16 years ago
EDITORBASE+
QA Contact: pmac → beppe
Whiteboard: EDITORBASE → EDITORBASE+

Updated

16 years ago
Keywords: testcase
(Assignee)

Comment 7

16 years ago
I think this looks like a nice bug to do my first steps with the editor code.
Can I take it?
(Assignee)

Comment 8

16 years ago
-> 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.
Assignee: mjudge → kaie
(Assignee)

Comment 9

16 years ago
Created attachment 117721 [details] [diff] [review]
Patch v1

This patch appears to fix the problem.
(Assignee)

Comment 10

16 years ago
jfrancis: Although you said you don't see the problem with NS 7.01, I do see it
with 7.01
(Assignee)

Updated

16 years ago
Attachment #117721 - Flags: review?(mjudge)

Comment 11

16 years ago
I can repro this one using the current trunk build on win32 (2003031804)
(Assignee)

Updated

16 years ago
Blocks: 192856

Comment 12

16 years ago
Comment on attachment 117721 [details] [diff] [review]
Patch v1

as long as you have tested it reasonably well
Attachment #117721 - Flags: review?(mjudge) → review+
(Assignee)

Comment 13

16 years ago
Comment on attachment 117721 [details] [diff] [review]
Patch v1

Kin, can you please sr?
Attachment #117721 - Flags: superreview?(kin)

Comment 14

16 years ago
Comment on attachment 117721 [details] [diff] [review]
Patch v1

sr=kin@netscape.com
Attachment #117721 - Flags: superreview?(kin) → superreview+

Comment 15

16 years ago
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?
(Assignee)

Comment 16

16 years ago
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?
(Assignee)

Comment 17

15 years ago
checked in, fixed
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 18

15 years ago
I have filed bug 200416, bug 200417, and bug 200418 which may be related to the
fix for this bug.
(Assignee)

Comment 19

15 years ago
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

15 years ago
reopening; this is a duplicate of 192320 which Kaie just fixed
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 21

15 years ago
oops; wrong bug!
Status: REOPENED → RESOLVED
Last Resolved: 15 years ago15 years ago
Resolution: --- → FIXED

Comment 22

15 years ago
verifying with my mac debug build from today
carryover keywords from dupe
Status: RESOLVED → VERIFIED
Keywords: topembed+
Whiteboard: EDITORBASE+ → EDITORBASE+ edt_x3

Updated

15 years ago
No longer blocks: 192856

Comment 23

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

Comment 24

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

Comment 25

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

Comment 26

15 years ago
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.
You need to log in before you can comment on or make changes to this bug.