299371 - Right arrow in a "select all" text field doesn't work

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Mark Mentovai]

This is most likely caused by bug 207186.  Regression between 06-30 03:00 EDT
and 23:45 EDT.

I am experiencing this on Macintosh.  It looks like it would apply to other
platforms as well.

In a text field that has its entire content selected, the right arrow key should
cancel the selection and move the insertion point to the end (right) of the text
in the field.

Now, right arrow keypresses are being ignored.

To reproduce: put some text in any text field (even the location bar or find
bar), Edit:Select All, and press the right arrow key.

Comment 1

[Eyal Rozenberg]

Seeing this bug with build 2005-06-29 #05, on the reply text field of bugzilla,
when I enter a single line of Hebrew text, make the field direction RTL,
select-all with Ctrl+A and press the right arrow (or the left arrow!!). So maybe
the checking for bug 207186 only exacerbated an existing problem. Not seeing
this bug when the text field is LTR.

Comment 2

[Benjamin Smedberg]

Putting on the 1.8b3 radar. Mano says he probably won't have a patch for this
before Sunday, so it seems that backing out is the safest option here.

Comment 3

[Mark Mentovai]

Confirming, bug 207186 was definitely the checkin at fault.

Comment 4

[Uri Bernstein]

Yes, I agree backing out is the best option for now. Sadly, I can't work on this
right now, and even if I could, it would take time for me to fix this and get a
patch appreved.

I apologize for causing this regression and for not being able to work on it
right away.

Comment 5

[Asaf Romano (gone)]

backed out.

Comment 6

[Uri Bernstein]

Mats, I'd like your help on this, since I believe my patch (which is now backed
out) merely exposed an existing problem in the code, specifically , code in
|nsSelection::MoveCaret()| related to bug 207623.

What happens is that my code causes |frame->PeekOffset()| (l. 1379) to fail in
the case of select-all+right-arrow in a text field. I believe this is a correct
behaviour, since there's really nowhere for the cursor to advance to in this
situation.
As a result, the |else if| on l. 1447 is entered. However, there is a problem
here: weakNodeUsed points to the nsHTMLDivElement containing the text. However,
offsetused is an offset into the TextNode itself (i.e. it is equal to the length
of the text). As a result, |nsTypedSelection::Collapse()| fails (inside the call
to |SetOriginalAnchorPoint|).

I tried fixing this by calling |mDomSelections[index]->Collapse()| with |node|
instead of |weakNodeRefernce|. This fixed the textbox case, but broke the
textarea case (which is, I think, what this code is all about).

My problem is that I don't know this code well enough. I suspect the correct
solution would somehow prevent entering that "else if" at all in the textbox
case, but I'm not sure.

Your help would be greatly appreciated.

Comment 7

[Uri Bernstein]

On the other hand, maybe my assumption that PeekOffset() is supposed (or at
least allowed)  to fail when there's nowhere to go, was wrong.
I wish there was some documentation somewhere detailing the input, output, and
expected behaviour of PeekOffset().