Closed Bug 305985 Opened 15 years ago Closed 15 years ago
after clicking in blank content area, focus stays in some widgets when it should have left
503 bytes, text/html
Remove unnecessary call to GetDocSelectionLocation (unrelated) and unecessary if conidtion for setting selection before current focused node (the fix)
2.24 KB, patch
|Details | Diff | Splinter Review|
180 bytes, text/html
1.29 KB, patch
|Details | Diff | Splinter Review|
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.
(Note that if you click on the "blocking1.9a1" *text*, it works as expected.)
Bug also occurs in Mozilla 2005-04-30 on Linux so it's not a recent regression.
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.)
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.
Confirming on OS X => All/All
OS: Linux → All
Hardware: PC → All
Does this also fix bug 308624?
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.
Attachment #196882 - Flags: review?(mats.palmgren) → review-
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'
(In reply to comment #10) > (From update of attachment 196882 [details] [diff] [review] ) > 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.
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.
Summary: focus stays in listbox select widget when it should have left → after clicking in blank content area, focus stays in some widgets when it should have left
*** Bug 308624 has been marked as a duplicate of this bug. ***
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 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.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [ETA: patch needs review] → [ETA: patch needs approval]
Attachment #197037 - Flags: approval1.8b5? → approval1.8b5+
Verified with Windows and Linux Firefox trunk builds 2005-09-23-07-trunk
Status: RESOLVED → VERIFIED
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.