305985 - after clicking in blank content area, focus stays in some widgets when it should have left

Status: VERIFIED FIXED

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

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Steps to reproduce:
 1. load bug 268497
 2. make the window wide enough so that you can see past the right edge of
    the flags comboboxes
 3. click in the "CC:" listbox (so you select somebody's email address)
 4. click in the blank space to the right of the combobox to the right of
    the text "blocking1.9a1" (or whatever the flags will be sometime in the
    future)
 5. press the down arrow

Expected results:
 5. page scrolls down

Actual results:
 5. "CC" listbox scrolls down

Firefox Linux trunk, morning of August 25.

I'm not sure yet if this is a regression, but I think it is.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Note that if you click on the "blocking1.9a1" *text*, it works as expected.)

Comment 2

[Mats Palmgren (inactive)]

Bug also occurs in Mozilla 2005-04-30 on Linux so it's not a recent regression.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Like bug 308624, this is a regression between suite Linux GTK1 builds
2004-12-01-08-trunk and 2004-12-02-11-trunk.  (Although in the latter, the
scrolling within the listbox was even wackier than it is now.)

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I confirmed by backout that this was caused by the fix for bug 269318 (which
still backs out cleanly with cvs up -j... -j...).  This is one of those bugs
that isn't necessarily obvious but makes the experience of using our software a
good bit worse, so nominating for blocking1.8b5.

Comment 5

[Uri Bernstein]

Confirming on OS X => All/All

Comment 6

[Aaron Leventhal]

Attachment: Simplified test case. Click in listbox, click to right of combo box, press down arrow

Comment 7

[Aaron Leventhal]

Attachment: Remove unnecessary call to GetDocSelectionLocation (unrelated) and unecessary if conidtion for setting selection before current focused node (the fix)

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Does this also fix bug 308624?

Comment 9

[Aaron Leventhal]

(In reply to comment #8)
> Does this also fix bug 308624?

Yes.

Comment 10

[Mats Palmgren (inactive)]

Comment on attachment 196882 [details] [diff] [review]
Remove unnecessary call to GetDocSelectionLocation (unrelated) and unecessary if conidtion for setting selection before current focused node (the fix)

I think the current code in MoveCaretToFocus() is actually correct.

'SelectNodeContents(node)' results in
    DoSetRange(node, 0, node, node.childCount);

'SetStartBefore(node)' results in
    DoSetRange(node.Parent, node.Parent.IndexOf(node), ...)

So the current code puts the caret "outside" if it's a leaf
and "inside" if not. What you propose would put it outside
always, which I think is wrong - see upcoming testcase.

The problem in the original testcase is that we fail to find any
content near the click.
The caret for that testcase should have been placed after the
second <select> and then the scroll problem would not occur.

Comment 11

[Mats Palmgren (inactive)]

Attachment: Testcase #2

1. Turn on caret browsing
2. type TAB once

Current result: caret is placed before the 'l' in 'line2'
Result with patch: caret is placed after the '1' in 'line1'

Comment 12

[Aaron Leventhal]

(In reply to comment #10)
> (From update of attachment 196882 [details] [diff] [review] [edit])
> The problem in the original testcase is that we fail to find any
> content near the click.
> The caret for that testcase should have been placed after the
> second <select> and then the scroll problem would not occur.
Yes, that could be considered a problem with fixing but I don't believe that's
the regression.

The regression is that we're selecting a form control's entire node when it is
not a leaf. This happens when you click inside a textarea or select. At that
point the document's caret should be right before the form control.

Comment 13

[Aaron Leventhal]

Attachment: Fix the regression part of the problem. Also fixes bug 308624.

Mats, I would like the "not finding content for the click" part of the problem
to be dealt with in a separate bug since it has been around for longer and
isn't the regression I caused from bug 269318.

Comment 14

[Aaron Leventhal]

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

Comment 15

[Aaron Leventhal]

This bug requires that you first click on a form control that is not a leaf,
such as a <textarea> or <select>

It was a regression from 269318, because because after a mouse click we now
MoveCaretToFocus() which is selecting the entire node unless it is a leaf. For a
leafd it it sets the caret right before the node.

We really want that leaf behavior for all form controls otherwise the entire
<textarea> or <select> gets selected when you click in it. Then the next time
you click in a blank area that doesn't correspond to a content node you're still
inside that prev widget.

Comment 16

[Brian Ryner (not reading)]

Comment on attachment 197037 [details] [diff] [review]
Fix the regression part of the problem. Also fixes bug 308624.

I'm not crazy about testing for HTML_FORM_CONTROL, but Aaron and I couldn't
come up with anything better that covers all of the cases.

Comment 17

[Tracy Walker [:tracy]]

Verified with Windows and Linux Firefox trunk builds 2005-09-23-07-trunk