289022 - Clicking to focus text inputs and textareas sometimes doesn't allow text entry.

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Wevah (Nate Weaver)]

In today's Camino build, when I click on a text input or textarea and attempt to
type, it instead starts find-as-you-type (I've set it to not require the initial
"/" in my user.js). It seems to happen most often when I double-click to select
a word, or click in the input/textarea again after typing.

Build ID: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2)
Gecko/20050404 Camino/0.8+

Comment 1

[Wevah (Nate Weaver)]

Hmm, this might be Camino-only; I can't reproduce with the latest Firefox nightly.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sounds like a focus issue, which would be Aaron

Comment 3

[Wevah (Nate Weaver)]

Some additional info: It seems to happen on even clicks. Click once, typing is
fine. Click again, typing breaks. Click again, typing is fine again. And on and
on...

Comment 4

[Simon Fraser [no longer active]]

Find-as-you-type prefs are not involed here. I can reproduce the problem in a
vanilla build.

To repro:
1. click in a textarea, and start typing. everything is fine.
2. click again in the same textarea. Now the caret disappears and you can't type.

Comment 5

[Simon Fraser [no longer active]]

Checkins:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-04-03+08%3A00%3A00&maxdate=2005-04-04+08%3A00%3A00&cvsroot=%2Fcvsroot

Comment 6

[Aaron Leventhal]

I have not made any focus related checkins in this regression window.

Most likely suspect I see in the regression window is Ginn Chen's checkin for
bug 260399 "disabled textarea/input should use DEFAULT pointer r+sr=bzbarsky"

2005-04-04 03:35	ginn.chen%sun.com 	mozilla/ content/ events/ src/
nsEventStateManager.h 	1.130 	0/1  	Bug 260399 disabled textarea/input should
use DEFAULT pointer r+sr=bzbarsky
2005-04-04 03:35	ginn.chen%sun.com 	mozilla/ content/ events/ src/
nsEventStateManager.cpp 	1.561 	6/34 
2005-04-04 03:35	ginn.chen%sun.com 	mozilla/ layout/ forms/ resources/ content/
xbl-forms.css 	1.27 	2/2 
2005-04-04 03:35	ginn.chen%sun.com 	mozilla/ layout/ style/ forms.css 	3.101 	4/2 

Comment 7

[Simon Fraser [no longer active]]

This might be related roc's changes.

When you click in the window, cocoa automatically sets the focussed NSView to
the clicked view (which, for a blank page, is the scroll port view's widget). We
then kick off Gecko focus logic, which changes the focus to the document view.
On every click, we do this swapping of the focussed widget.

Comment 8

[Simon Fraser [no longer active]]

I see us hitting a code path in PresShell::HandleEvent() that seems wrong. For a
textarea, the keypress comes in via the nsScrollPortView's widget. We hit
PresShell::HandleEvent(), which does:

  nsIFrame* frame = NS_STATIC_CAST(nsIFrame*, aView->GetClientData());

the scroll port view has no frame (is that new?), so we drop into the
RetargetEventToParent() code near the bottom of that method. That then sets
focus to the DocumentViewer's widget, thus breaking typing.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #8)
> I see us hitting a code path in PresShell::HandleEvent() that seems wrong. For
> a textarea, the keypress comes in via the nsScrollPortView's widget.

OK

> We hit PresShell::HandleEvent(), which does:
> 
>   nsIFrame* frame = NS_STATIC_CAST(nsIFrame*, aView->GetClientData());
> 
> the scroll port view has no frame (is that new?)

Yes indeed.

> so we drop into the RetargetEventToParent() code near the bottom of that
> method. That then sets focus to the DocumentViewer's widget, thus breaking
> typing.

I guess the easiest fix is to hack the viewmanager so that it retargets such
events. Around here:
http://lxr.mozilla.org/mozilla/source/view/src/nsViewManager.cpp#2459
if aView->GetClientData() is null, we should scan upwards through parents
looking for a view with client data and use that instead.

Comment 10

[Simon Fraser [no longer active]]

Attachment: Retarget events on frameless views to parent with frame.

Comment 11

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

Comment on attachment 179663 [details] [diff] [review]
Retarget events on frameless views to parent with frame.

It seems like this code should be in the pres shell, not the view manager. 
Views shouldn't treat the nsIViewObserver API differently depending on the
opaque "client data" pointer.

Also, should there be a coordinate transformation here?  Does this depend on
the extra views being at (0,0)?  (Are they?)

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, this would be better in the PresShell.

These are key events so the coordinates are meaningless.

Comment 13

[Simon Fraser [no longer active]]

Comment on attachment 179663 [details] [diff] [review]
Retarget events on frameless views to parent with frame.

Sounds like you don't want this patch.

Comment 14

[Simon Fraser [no longer active]]

We need this fixed before the tree closes for alpha. It makes Camino almost
unusable with forms.

Comment 15

[Simon Fraser [no longer active]]

Attachment: Incomplete patch for PresShell

This patch has a couple of XXX comments in it. When retargeting events to
parent views:
  Do we need to fix up coords for IME events?
  Is it ok to change |aView| for the rest of the method.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #15)
> This patch has a couple of XXX comments in it. When retargeting events to
> parent views:
>   Do we need to fix up coords for IME events?

Might as well, it's easy. Just do
    aEvent->point += aView->GetOffsetTo(targetView);
    aView = targetView;

>   Is it ok to change |aView| for the rest of the method.

I think so.

Comment 17

[Simon Fraser [no longer active]]

Attachment: Patch nsPresShell::HandleEvent

Comment 18

[Wevah (Nate Weaver)]

Changing the wording of the subject a bit to include the word "focus", which
seems to be the most common word used when looking for this (silly me).

Comment 19

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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

Comment 20

[Wevah (Nate Weaver)]

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

Comment 21

[Simon Fraser [no longer active]]

Request blocking status. This bug makes camino unusable.

Comment 22

[Simon Fraser [no longer active]]

Mine.

Comment 23

[Simon Fraser [no longer active]]

Checked in to trunk.

Comment 24

[Wevah (Nate Weaver)]

Verified.

Comment 25

[Hiro]

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