284163 - [FIX]display:none form controls cannot be toggled/focused via label

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Martijn Wargers (dead)]

I think bug 231081 regressed.

See testcase from that bug (which is the url testcase of this bug).
-click once on the "toggle display radio/checkbox buttons" to make the
checkboxes/radio buttons display:none.
-After that, click on of the labels "radio #r1", etc.
The background color of the clicked label should turn to green.
This doesn't happen in current trunk builds.

This works in the 20050119 trunk build.
It doesn't work in the 20050120 trunk build.
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-01-19+16%3A07&maxdate=2005-01-20+22%3A16&cvsroot=%2Fcvsroot
I guess the fix for bug 244366 seems a likely candidate for this regression.

Comment 1

[Boris Zbarsky [:bzbarsky]]

That bug was about the radio buttons being selected but style not changing.

In a current build, the radio buttons are not selected (and hence, of course),
the style is not changed.  So no style reresolution issue here...  Changing
summary accordingly.

Now as to when this behavior changed... I can toggle the controls in a
2005-01-20-08 build, but not a 2005-01-21-18 one.  So (for once) bug 244366 is
off the hook.  ;)

The behavior has changed due to bug 146066.  It looks like
HandleEventWithTarget() doesn't deal with frame-less targets, whereas
HandleDOMEventWithTarget() does.

So questions:

1)  Should clicking on the label for a display:none control toggle its state? (I
    think the answer is "yes").
2)  Should HandleEventWithTarget() dispatch the event to content when there is
    no frame?  (Again, the answer, imo, is "yes").  Currently there is a
    NS_EVENT_NEEDS_FRAME() check in the method, but this is subverted by later
    null-checks on GetCurrentEventFrame() around the dispatch to DOM...

ccing some people who may know something about labels and event dispatch.

Note that I can fix #1 by just not using HandleEventWithTarget() here, but I 
think the distinction between the two methods is very artificial and we should
just make HandleEventWithTarget() work for this case and ditch
HandleDOMEventWithTarget() altogether.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Simple "fix"

This reverts to the old "content-only" behavior while we sort out the general
issue...

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 175866 [details] [diff] [review]
Simple "fix"

r+sr=jst

Comment 4

[Boris Zbarsky [:bzbarsky]]

Checked  the patch in, restoring the status quo.  I'd still like to know whether
anyone knows the answers to my questions from comment 1 (either here, or in
netscape.public.mozilla.dom).