Closed Bug 6747 Opened 25 years ago Closed 25 years ago

HTML4 LABEL works OK in HTML but not in XUL

Categories

(Core :: XUL, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: german, Assigned: pollmann)

References

Details

(Whiteboard: fix in my tree)

Attachments

(4 files)

I checked to make sure it's not a dupe - sorry if it is. XUL seems to ignore HTML 4 label elements that work fine as HTML . Need this asap for building dialogs. Test cases will be attached momentarily
Assignee: trudelle → hyatt
reassigning to hyatt
Status: NEW → ASSIGNED
Target Milestone: M6
I also have a fix for this. I'd like to check it in for M6. cc'ing chofmann.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Fixed. You don't need the html:form in the test case.
QA Contact: gerardok → ckritzer
Changing QA Contact back to ckritzer@netscape.com.
Status: RESOLVED → REOPENED
This does not work anymore in m11, m12 when used in XUL, and only patially for the text fields when used in HTML. By 'odes not work' I mean that the checkboxes, radiobuttons etc do not get the focus anymore when a user clicks on the label. ------ Here is the HTML code: <form> <br> <label for="fname" accesskey="f">First Name</label> <input type="text" name="firstname" id="fname"> <br> <label for="lname" accesskey="l">Last Name</label> <input type="text" name="lastname" id="lname"> <br> <input type="radio" name="t" id="r1" checked="true"> <label for="r1" accesskey="1">Test 1</label> <br> <input type="radio" name="t" id="r2"> <label for="r2" accesskey="2">Test 2</label> <br> </form> ------ Here is the XUL code: <?xml version="1.0"?> <!DOCTYPE window> <window xmlns:html="http://www.w3.org/TR/REC-html40" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> <html:form> <html:br/> <html:label for="fname" accesskey="f">First Name</html:label> <html:input type="text" name="firstname" id="fname"/> <html:br/> <html:label for="lname" accesskey="l">Last Name</html:label> <html:input type="text" name="lastname" id="lname"/> <html:br/> <html:input type="radio" name="t" id="r1" checked="true"/> <html:label for="r1" accesskey="1">Test 1</html:label> <html:br/> <html:input type="radio" name="t" id="r2"/> <html:label for="r2" accesskey="2">Test 2</html:label> <html:br/> </html:form> </window>
Clearing FIXED resolution due to reopen.
Resolution: FIXED → ---
Target Milestone: M6 → M13
targetting m13
spam: changing qa contact from ckritzer -> paulmac for xul bugs
Assignee: hyatt → pollmann
Status: REOPENED → NEW
Re-assigning to pollmann. cc'ing waterson, who should ensure the hidden form still wraps us.
I had patched nsLabelFrame.cpp to look for XUL documents and behave accordingly. Maybe that code got changed?
Labels don't work in HTML or XUL right now, they were broken on 11-Aug when someone removed the MouseClicked method from checkboxes and radios. Now the label frame calls the MouseClicked method, but nothing happens. :S See bug 16810. I'd guess that this is a dup of 16810...
Status: NEW → ASSIGNED
Depends on: 16810
OS: other → All
Hardware: PC → All
Target Milestone: M13 → M14
I'm adding bug 16810 as a dependency of this bug so we can check again after html labels are working. Marking M14 as that's what 16810 is marked.
BULK MOVE: Changing component from XUL to XP Toolkit/Widgets: XUL. XUL component will be deleted.
Component: XUL → XP Toolkit/Widgets: XUL
Hyatt's patches to nsLabelFrame.cpp are still in place and functioning correctly. The problem is that the mouse click isn't even getting to the nsLabelFrame::HandleEvent() for some reason. I'm looking at it.
The culprit appears to be nsHTMLLabelElement::HandleDOMEvent(). I will need to make changes very much parallel to hyatts changes in nsLabelFrame::GetForControl() to allow events to go from the content to the frames here. (I was assuming that the document was an html document but I will have to add a qi for a xul document if that fails). Will try to complete this and check it in tomorrow.
I have a fix, so I am taking the bug. The problem is the SetFocus doesn't do anything, you need to set focus via the eventstatemanager: case NS_MOUSE_LEFT_BUTTON_DOWN:{ nsIEventStateManager *stateManager; if (NS_SUCCEEDED(aPresContext->GetEventStateManager(&stateManager))) { nsIContent * content; mControlFrame->GetFormContent(content); if (nsnull != content) { stateManager->SetContentState(mContent, NS_EVENT_STATE_FOCUS); } NS_RELEASE(content); NS_RELEASE(stateManager); } //mControlFrame->SetFocus(PR_TRUE); mLastMouseState = eMouseDown; *aEventStatus = nsEventStatus_eConsumeNoDefault; } break;
Assignee: pollmann → rods
Status: ASSIGNED → NEW
Whiteboard: fix in my tree
accepting bug
Status: NEW → ASSIGNED
rods: don't you want the NS_RELEASE(content) to be inside the if (content != nsnull) check?
Oops, Sure. (I just added that extra check) Turns out the better solution is: case NS_MOUSE_LEFT_BUTTON_DOWN:{ nsIContent * content; mControlFrame->GetFormContent(content); if (nsnull != content) { nsCOMPtr<nsIFocusableContent> focusable(do_QueryInterface(content)); if (focusable) { focusable->SetFocus(aPresContext); } NS_RELEASE(content); } mLastMouseState = eMouseDown; *aEventStatus = nsEventStatus_eConsumeNoDefault; } break;
Chris the other bug I sent you is really a duplicate of this bug. Except that we really don't have a bug for fixing the focus issue. The code below fixes the focus issue which I will check in tomorrow.
Cool - thanks Rod. (In addition to your fix, which is the real bug reported here, I'll still have to make the changes I mentioned to get javascript events passed from the label to the control. See bug 7554 for more info. I'll post here when I get that checked in.)
It seems that we are overlapping effort from the content and frame side. When I fixed the bug where javascript events weren't getting processed, it also caused this bug (6747) to go away, without the patch to the label frame mentioned here. This probably means that we should either stop pushing events from the label to the piece that it is for - either from the frame or from the content. I don't really care which. I'll attach a patch (applies to nsHTMLLabelElement.cpp) this fixes both this bug and a bug demonstrated by another test case (will attach additional test case).
I agree that the frame HandleEvent should NOT be doing anything. So I ifdef'ed out all the code there and it still works fine. It can be removed soon, I checke it in with the "ifdef 0" tomorrow I will remove the entire HandleEvent.
Rod can you review the 'Alternate fix' attached so I can check it in? Thanks!
Doing a reverse grab as I'm checking in the fix. :)
Assignee: rods → pollmann
Status: ASSIGNED → NEW
Checked in the fix. This will have to change when GetElementById is implemented on nsIDocument. To verify: 1) Go to testcase labeled "XUL file showing HTML4 labels not working in XUL". Clicking on the text next to the checkboxes and textfields should be equivalent to clicking on the checkboxes and textfields themselves. 2) Go to testcase labeled "Test case, click on label should dump "clicked!\n"" Clicking on the label should print "clicked" to the console and act as clicking on the element does.
Status: NEW → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
In the "XUL file showing HTML4 labels not working in XUL" test case, radio button labels work - but text field labels don't. Reopening. I'm using the 2000022216 build on WinNT4 SP6a.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Bummer, this must have regressed sometime in the past month.
Status: REOPENED → ASSIGNED
Target Milestone: M14 → M15
Playing with the test cases I noticed that the XUL document is behaving exactly like the HTML document, so this bug is not reopened. Perhaps we should open up a more general bug "labels don't work". I also noticed that if a text field (A) has focus, and a label for the other text field (B) is clicked on, text field (A) looses focus, but (B) doesn't get focus. It seems to 'partly' work. Also noticed that if the label for the radio button is clicked on, the selection dot appears in the radio button, but the radio button does not appear depressed unless you actually click on the button itself. Again, the label 'partly' works. It seems like we are dividing up the responsibility of handling mouse and focus events between content and frames a little too loosely. I'll open up a new bug. This one should be closed though, as HTML labels in XUL documents do still work the same as HTML labels in HTML documents.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago25 years ago
Resolution: --- → FIXED
New report is bug 29662.
Target Milestone: M15 → M14
please ignore, massive spam giving jrgm@netscape.com backlog of XPToolkits resolved fixed bugs to verify
QA Contact: paulmac → jrgm
pollmann is correct: label behaviour is identical in the XUL & HTML test cases. Verified fixed on Win98 build 2000-04-09-08 - will verify soon on Mac and Linux, as well.
Verified Fixed on Mac & Linux (2000-04-10-08 builds). pollman, you wrote earlier: > I also noticed that if a text field (A) has focus, and a label for the other > text field (B) is clicked on, text field (A) looses focus, but (B) doesn't get > focus. It seems to 'partly' work. This doesn't actually mean that it 'partly works', since it's normal behaviour to lose focus on a text field when _any_ other part of the window is clicked - be it another text field or just a blank space.
Status: RESOLVED → VERIFIED
Just to clarify what I meant to say: > This doesn't actually mean that it 'partly works', since it's normal behaviour > to lose focus on a text field when _any_ other part of the window is clicked - > be it another text field or just a blank space. Yes, that's exactly what I meant. It is normal behaviour to loose focus on a text field when _any_ other part of the window is clicked. That part of the expected behaviour does indeed work! :) That means that clicking on the label is going through part of the focus system that causes the focus to be removed from the text field, but is not transfering focus to the object the label is for. 'partly working' was just an expression that it is not completely broken - as it would be if clicking on the label did _nothing_. This fix is scheduled for M17, so hopefully we'll see it fully working soon!
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: