[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: [PATCH] Last version of the "fix label form focus" (59489)
From:       bj () altern ! org
Date:       2004-07-07 18:18:39
Message-ID: 200407072025.54321.bj () altern ! org
[Download RAW message or body]

Hi!

> I looked at your patch, and though I'm not really fluent with the event
> handling mechanism, there are things that bother me:
> - first: indentation is crappy, please fix it to match the code style :-)
> - you seem to short-circuit totally the DOM events with your activateInput
> ...it should really be a click()
> - your reimplementation of defaultEventHandler looks wrong too
>   -> does not call the base class (how will it handle other events then)
>   -> does not mark the event as defaultHandled
>
> eventually, I wonder if your activateInput isn't just turning around a
> shortcoming in HTMLInputElementImpl::defaultEventHandler when receiving  a
> CLICK_EVENT without a previous MOUSE_UP

Most problems corrected in this new patch. You were right about the 
CLICK_EVENT without a MOUSE_UP event. So I changed the 
HTMLInputElementImpl::defaultEventHandler to react on CLICK_EVENT rather than 
on MOUSE_UP event. Could'nt find any side effect, but maybe someone can give 
me a reason why it is better to use MOUSE_UP...

Otherwise, it is much cleaner now. OK?

["forms.diff" (text/x-diff)]

Index: khtmlview.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/khtmlview.cpp,v
retrieving revision 1.640
diff -u -3 -r1.640 khtmlview.cpp
--- khtmlview.cpp	30 Jun 2004 16:20:34 -0000	1.640
+++ khtmlview.cpp	7 Jul 2004 18:13:31 -0000
@@ -1885,7 +1885,7 @@
             static_cast< HTMLAreaElementImpl* >( node )->click();
           break;
         case ID_LABEL:
-            // TODO should be click(), after it works for label
+            static_cast< HTMLLabelElementImpl* >( node )->click();
           break;
         case ID_TEXTAREA:
           break; // just focusing it is enough
Index: html/html_formimpl.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/html/html_formimpl.cpp,v
retrieving revision 1.372
diff -u -3 -r1.372 html_formimpl.cpp
--- html/html_formimpl.cpp	22 Jun 2004 05:46:41 -0000	1.372
+++ html/html_formimpl.cpp	7 Jul 2004 18:13:32 -0000
@@ -1481,19 +1481,22 @@
     {
 
         if (evt->isMouseEvent()) {
-	    MouseEventImpl *me = static_cast<MouseEventImpl*>(evt);
             if ((m_type == RADIO || m_type == CHECKBOX)
-		&& me->id() == EventImpl::MOUSEUP_EVENT && me->detail() > 0) {
+		&& evt->id() == EventImpl::CLICK_EVENT) {
 		// click will follow
 		setChecked(m_type == RADIO ? true : !checked());
+		focus();  // nedded for when the input is avtivated via its label
 	    }
             if (evt->id() == EventImpl::CLICK_EVENT && m_type == IMAGE && m_render) \
{  // record the mouse position for when we get the DOMActivate event
 		int offsetX, offsetY;
+		MouseEventImpl *me = static_cast<MouseEventImpl*>(evt);
 		m_render->absolutePosition(offsetX,offsetY);
 		xPos = me->clientX()-offsetX;
 		yPos = me->clientY()-offsetY;
 	    }
+	    if ((m_type == TEXT || m_type == PASSWORD || m_type == FILE) && \
evt->id()==EventImpl::CLICK_EVENT) +	    focus();
 	}
 
         if (m_type == RADIO || m_type == CHECKBOX || m_type == SUBMIT || m_type == \
RESET || m_type == BUTTON ) { @@ -1580,15 +1583,37 @@
     HTMLElementImpl::attach();
 }
 
-#if 0
-ElementImpl *HTMLLabelElementImpl::formElement()
+void HTMLLabelElementImpl::click()
 {
-    DOMString formElementId = getAttribute(ATTR_FOR);
-    if (formElementId.isEmpty())
-        return 0;
-    return getDocument()->getElementById(formElementId);
+    QMouseEvent me(QEvent::MouseButtonRelease, QPoint(0,0),Qt::LeftButton, 0);
+    dispatchMouseEvent(&me,EventImpl::CLICK_EVENT, 1);
 }
-#endif
+
+void HTMLLabelElementImpl::defaultEventHandler(EventImpl *evt)
+ {
+    if (( !m_disabled ) && (evt->isMouseEvent()))
+        if (evt->id() == EventImpl::CLICK_EVENT){
+	    DOMString formElementId = getAttribute(ATTR_FOR);
+	    bool found=false;
+    	    if (!formElementId.isEmpty())
+    	        if (static_cast<DOM::HTMLInputElementImpl*>(getDocument()->getElementById(formElementId))){
 +		    static_cast<DOM::HTMLInputElementImpl*>(getDocument()->getElementById(formElementId))->click();
 +		    found=true;
+		    evt->setDefaultHandled();
+		}
+    	    if (!found){
+    		uint children=childNodeCount();
+    		if (children>1) 
+    		for (unsigned int i=0;i<children;i++)
+    		if (childNode(i)->id()==ID_INPUT){
+    		    static_cast<DOM::HTMLInputElementImpl*>(childNode(i))->click();
+		    evt->setDefaultHandled();
+    		    break;
+    		}
+    	    }
+        }
+    HTMLGenericFormElementImpl::defaultEventHandler(evt);
+ }
 
 // -------------------------------------------------------------------------
 
Index: html/html_formimpl.h
===================================================================
RCS file: /home/kde/kdelibs/khtml/html/html_formimpl.h,v
retrieving revision 1.152
diff -u -3 -r1.152 html_formimpl.h
--- html/html_formimpl.h	22 Jun 2004 05:46:41 -0000	1.152
+++ html/html_formimpl.h	7 Jul 2004 18:13:32 -0000
@@ -319,6 +319,8 @@
 
     virtual Id id() const;
     virtual void attach();
+    virtual void defaultEventHandler(EventImpl *evt);
+    void click();
 
  private:
     DOMString m_formElementID;



[prev in list] [next in list] [prev in thread] [next in thread]