49897 - Label events: Context menu for radio/checkbox labels disables keyboard input

Status: VERIFIED FIXED

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

Description

[Blake Ross]

Build ID: 2000082208

Steps to Reproduce:

(1) Right click on "hello" in the testcase I'll attach
(2) Click elsewhere to dismiss the context menu.

Note that the context menu doesn't go away (even when switching apps) until you 
choose an item or press escape. This is a weird one...

(You can also see this in Debug | Viewer Demos | #8 Form...scroll down to 
the "html 4 labels" area and right click on the checkbox/radio button labels 
there, then try to dismiss the context menu).

Comment 1

[Blake Ross]

Attachment: testcase

Comment 2

[R.K.Aa.]

On linux 2000082208 the context menu does go away but this error displays in
console when i click elsewhere in the testcase page:

Gdk-CRITICAL **: file gdkwindow.c: line 1479 (gdk_window_get_origin): assertion
`window != NULL' failed.

Worse though: After clicking back-button and trying to comment in this bug, the
keyboard was almost dead: couldn't write ordinary text, only national characters
worked. Had to close mozilla and restart to write comment.

Tested this several times, also after fresh restart - it happened each time.

Comment 3

[Blake Ross]

Note that if you click on a selection in the label, the context menu behaves 
normally (and dismisses when it should).

Comment 4

[John Morrison]

The popup dismisses for me, in all cases, on win2k/win98 and on mac with 
today's build (e.g. when I click elsewhere in the same window, when I click
on another window, when I hit ESC ...).

On linux, it dismisses if I click elsewhere in the same window (although
that causes the Gdk-CRITICAL message). ESC causes the popup to dismiss without 
the erro. However, if I click on another window's titlebar to raise it up, the 
mozilla context menu stays at the top level (and this is with either focus 
follows mouse, or focus follows pointer click).

This only happens for <label>; I believe it's because this element gets special
handling (so that a click on the <label> gets delivered to it's companion form
control). 

I also see the assertion on linux. 

Comment 5

[Blake Ross]

hmm.  I think I was wrong earlier wrt what I said about if text is selected or 
not, since now the context menu still won't go away for me even if I've 
highlighted some text.  Still reproducible for me consistently, though.

Comment 6

[Blake Ross]

By the way, I, too, see the disabling and weird behavior of the keyboard. 
(sorry 'bout the spam, last time for tonight)

Comment 7

[Mike Pinkerton (not reading bugmail)]

i tried this on mac with a build from late last week and it worked just fine 
(note, i tried the viewer 8 demo, not the submitted attachment).

so what do we do about this one?

Comment 8

[John Morrison]

While I can only reproduce the 'wont go away' context menu on Linux, I can,
on mac/linux/win32 reproduce the other problem -- after bringing up that 
context menu over the <label> I can no longer type in any textfield (either 
the URL bar or in a text control in the HTML page). (Actually some of the 
keypresses are now being routed to the menu accelerators (e.g., 'f'))

<label for=radio1>hello</label>
<input type=radio name=radio1><br>
<input type=text><textarea>foo bar</textarea>

So, basically, a user can completely break a form page if they click in the 
wrong place. (The question is how common is this scenario (e.g., right now 
the <label> is not widely used, but that may be changing). This same bug has
been around for a while -- an Aug 02 mozilla build exhibits the same behaviour).

Comment 9

[Peter Trudelle]

Yeah, this kills the window for good, and that is real bad since there could be
data loss scenarios based on this.

nsbeta3+ to stop the keyboard from becoming disabled, P2 for M18

Comment 10

[saari (gone)]

Make label stop tossing anything other than a left click event. 

Comment 11

[Mike Pinkerton (not reading bugmail)]

the xul popup listener is getting _two_ mouse up events, so we install two 
keyboard listeners. only one gets removed. that's why this is happening.

however, why we're getting two mouseUp events is unknown to me, and probably has 
something to do with the label element's fancy event dispatching.

cc'ing pollmann. this is probably his bug, but i'll keep looking a little.

Comment 12

[Mike Pinkerton (not reading bugmail)]

there is a call at both the top and bottom of 
nsHTMLLabelElement::HandleDOMEvent() to dispatch the DOM event. Both cause the 
event to bubble back up to the browser who is listening for the mouseUp, hence 
the two mouseUp events. 

Not feeling like mucking with code that I don't understand, passing this off to 
pollmann.

Comment 13

[Eric Pollmann]

I remember this bug...  I think it's pretty nasty.  We have to break at least
one of three things (label doesn't respond to events, content label is for
doesn't respond to events, or we get two events).

The only other solution I remember thinking of is to add code to be able to
merge two events to one as they bubble up, but I have no idea how to implement
this or even if it is a reasonable thing to do.  CC'ing Tom and Heikki to see if
they remember or have ideas about this.

As a start, we should probably take Saari's advice and only pass over events
from the label if they are left mouse events.

Comment 14

[Eric Pollmann]

Actually, according to the spec:
http://www.w3.org/TR/REC-html40/interact/forms.html#edef-LABEL

We should only pass focus on to the label's target.  I'll dig around and see
what Nav and IE do to see if this is reasonable.

Comment 15

[Eric Pollmann]

In my tree, I've taken out the code that passes on many random event types, and
instead restricted this to just NS_FOCUS_CONTENT (per HTML 4.0 17.9.1) and
NS_MOUSE_LEFT_CLICK (per IE's quirky behaviour and bugs 7292 and 7554).  I'll
check this in when the tree opens.

Comment 16

[Eric Pollmann]

Fix checked in.  To verify:

- View resource:///res/samples/test8.html
- Scroll down to the first radio
- Right click on the text to the left "radio 1 label"
- Click elsewhere on the page to dismiss the context menu
- Click in the URL bar and type a URL to visit.
- Press Enter

You should be able to type in the URL bar and go to another page.  Thanks all
for the great info on this bug report!

Comment 17

[John Morrison]

verified fixed mac/linux/win32 20000901nn builds. I can dismiss the context
menu in all the usual ways on all platforms, after I get the context menu I 
can enter text in html text controls and the URL bar and I no longer get
the gdk assertion on linux. Marking verified -- but Blake, did this clear up the 
'cannot dismiss context menu' as well (I could never reproduce that).