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

List:       kfm-devel
Subject:    [PATCH] #69559
From:       Lubos Lunak <l.lunak () suse ! cz>
Date:       2003-12-05 14:17:42
[Download RAW message or body]

Hello,

 ok, another try :). As Qt keyrelease events don't have text() set for some 
reason (possibly a bug, I'll report it), the patch solves the problem with 
two Qt autorepeat events generating only one DOM keypress events by 
postponing the first Qt keyrelease event, and it will be either entirely 
discarded or replayed depending on the following Qt autorepeat keypress. It 
looks a bit tricky, but I think the replaying doesn't affect anything.

 Also, there are no strange hack when translating Qt events to TextEventImpl, 
the event is simply left as it is, and an extra bool is added to 
NodeImpl::dispatchKeyEvent() is added to mark DOM keypress. It actually shows 
one place in RenderWidget where Qt events are translated to DOM ones 
improperly, but I haven't managed to actually find out if that code is used 
at all - I didn't reach it during testing.

 Oh, and this time I also didn't forget the reverse mapping in 
RenderWidget ;).

 The second patch has two changes where I think incorrect DOM events are used 
WRT holding a key down.

-- 
Lubos Lunak
KDE developer
---------------------------------------------------------------------
SuSE CR, s.r.o.  e-mail: l.lunak@suse.cz , l.lunak@kde.org
Drahobejlova 27  tel: +420 2 9654 2373
190 00 Praha 9   fax: +420 2 9654 2374
Czech Republic   http://www.suse.cz/

["khtml.patch" (text/x-diff)]

--- khtml/xml/dom_nodeimpl.cpp.sav	2003-10-30 13:18:29.000000000 +0100
+++ khtml/xml/dom_nodeimpl.cpp	2003-12-04 22:42:34.000000000 +0100
@@ -675,11 +675,11 @@ bool NodeImpl::dispatchSubtreeModifiedEv
 			 true,false,0,DOMString(),DOMString(),DOMString(),0),exceptioncode,true);
 }
 
-bool NodeImpl::dispatchKeyEvent(QKeyEvent *key)
+bool NodeImpl::dispatchKeyEvent(QKeyEvent *key, bool keypress)
 {
     int exceptioncode = 0;
     //kdDebug(6010) << "DOM::NodeImpl: dispatching keyboard event" << endl;
-    TextEventImpl *keyEventImpl = new TextEventImpl(key, \
getDocument()->defaultView()); +    TextEventImpl *keyEventImpl = new \
TextEventImpl(key, keypress, getDocument()->defaultView());  keyEventImpl->ref();
     bool r = dispatchEvent(keyEventImpl,exceptioncode,true);
     // the default event handler should accept() the internal QKeyEvent
--- khtml/xml/dom_nodeimpl.h.sav	2003-10-27 20:30:26.000000000 +0100
+++ khtml/xml/dom_nodeimpl.h	2003-12-04 22:42:18.000000000 +0100
@@ -255,7 +255,7 @@ public:
     bool dispatchMouseEvent(QMouseEvent *e, int overrideId = 0, int overrideDetail = \
0);  bool dispatchUIEvent(int _id, int detail = 0);
     bool dispatchSubtreeModifiedEvent();
-    bool dispatchKeyEvent(QKeyEvent *key);
+    bool dispatchKeyEvent(QKeyEvent *key, bool keypress);
 
     void handleLocalEvents(EventImpl *evt, bool useCapture);
 
--- khtml/xml/dom2_eventsimpl.cpp.sav	2003-12-04 22:36:40.000000000 +0100
+++ khtml/xml/dom2_eventsimpl.cpp	2003-12-04 22:42:07.000000000 +0100
@@ -423,7 +423,7 @@ TextEventImpl::TextEventImpl()
   qKeyEvent = 0;
 }
 
-TextEventImpl::TextEventImpl(QKeyEvent *key, AbstractViewImpl *view)
+TextEventImpl::TextEventImpl(QKeyEvent *key, bool keypress, AbstractViewImpl *view)
   : UIEventImpl(KHTML_KEYDOWN_EVENT,true,true,view,0)
 {
   qKeyEvent = new QKeyEvent(key->type(), key->key(), key->ascii(), key->state(), \
key->text(), key->isAutoRepeat(), key->count() ); @@ -432,8 +432,7 @@ \
TextEventImpl::TextEventImpl(QKeyEvent *  // (and e.g. space would make it scroll \
down)  //qKeyEvent->ignore();
 
-  // see KHTMLView::dispatchKeyEvent()
-  if( key->isAutoRepeat())
+  if( keypress )
     m_id = KHTML_KEYPRESS_EVENT;
   else if( key->type() == QEvent::KeyPress )
     m_id = KHTML_KEYDOWN_EVENT;
--- khtml/xml/dom2_eventsimpl.h.sav	2003-10-30 13:18:28.000000000 +0100
+++ khtml/xml/dom2_eventsimpl.h	2003-12-04 22:41:39.000000000 +0100
@@ -260,7 +260,7 @@ public:
 	       bool inputGeneratedArg,
 	       bool numPadArg);
 
-  TextEventImpl(QKeyEvent *key, AbstractViewImpl *view);
+  TextEventImpl(QKeyEvent *key, bool keypress, AbstractViewImpl *view);
 
   virtual ~TextEventImpl();
 
--- khtml/rendering/render_replaced.cpp.sav	2003-12-02 15:11:55.000000000 +0100
+++ khtml/rendering/render_replaced.cpp	2003-12-04 23:09:03.000000000 +0100
@@ -469,7 +469,9 @@ bool RenderWidget::eventFilter(QObject* 
         break;
     case QEvent::KeyPress:
     case QEvent::KeyRelease:
-        if (!element()->dispatchKeyEvent(static_cast<QKeyEvent*>(e)))
+    // TODO this seems wrong - Qt events are not correctly translated to DOM ones,
+    // like in KHTMLView::dispatchKeyEvent()
+        if (!element()->dispatchKeyEvent(static_cast<QKeyEvent*>(e),false))
             filtered = true;
         break;
     default:
@@ -590,18 +592,19 @@ bool RenderWidget::handleEvent(const DOM
         // See KHTMLView::dispatchKeyEvent: autorepeat is just keypress in the DOM
         // but it's keyrelease+keypress in Qt. So here we do the inverse mapping as
         // the one done in KHTMLView: generate two events for one DOM auto-repeat \
                keypress.
-        // (We know it's auto-repeat if the Qt event is KeyRelease)
+        // Similarly, DOM keypress events with non-autorepeat Qt event do nothing \
here, +        // because the matching Qt keypress event was already sent from DOM \
keydown event.  
         // Reverse drawing as the one in KHTMLView:
         //  DOM:   Down     Press   |       Press                             |     \
                Up
         //  Qt:    Press  (nothing) | Release(autorepeat) + Press(autorepeat) |   \
Release  
         QKeyEvent *ke = static_cast<const TextEventImpl &>(ev).qKeyEvent;
-        if (ke && ke->type() == QEvent::KeyRelease) {
+        if (ke && ke->isAutoRepeat()) {
+            QKeyEvent releaseEv( QEvent::KeyRelease, ke->key(), ke->ascii(), \
ke->state(), +                               ke->text(), ke->isAutoRepeat(), \
ke->count() ); +            static_cast<EventPropagator \
*>(m_widget)->sendEvent(&releaseEv);  static_cast<EventPropagator \
                *>(m_widget)->sendEvent(ke);
-            QKeyEvent pressEv( QEvent::KeyPress, ke->key(), ke->ascii(), \
                ke->state(),
-                               ke->text(), true /*autorepeat*/, ke->count() );
-            static_cast<EventPropagator *>(m_widget)->sendEvent(&pressEv);
         }
 
 	break;
--- khtml/khtmlview.cpp.sav	2003-12-02 15:11:50.000000000 +0100
+++ khtml/khtmlview.cpp	2003-12-04 22:43:09.000000000 +0100
@@ -125,7 +125,7 @@ public:
         prevScrollbarVisible = true;
 	tooltip = 0;
         possibleTripleClick = false;
-        filter_next_autorepeat_press = false;
+        postponed_autorepeat = NULL;
     }
     ~KHTMLViewPrivate()
     {
@@ -133,6 +133,7 @@ public:
         delete tp; tp = 0;
         delete paintBuffer; paintBuffer =0;
         delete vertPaintBuffer;
+        delete postponed_autorepeat;
         if (underMouse)
 	    underMouse->deref();
 	delete tooltip;
@@ -168,6 +169,8 @@ public:
 	clickCount = 0;
 	isDoubleClick = false;
 	scrollingSelf = false;
+        delete postponed_autorepeat;
+        postponed_autorepeat = NULL;
 	layoutTimerId = 0;
         repaintTimerId = 0;
         scrollTimerId = 0;
@@ -244,7 +247,6 @@ public:
     bool borderTouched:1;
     bool borderStart:1;
     bool scrollBarMoved:1;
-    bool filter_next_autorepeat_press:1;
 
     QScrollView::ScrollBarMode vmode;
     QScrollView::ScrollBarMode hmode;
@@ -262,6 +264,7 @@ public:
     int prevMouseX, prevMouseY;
     bool scrollingSelf;
     int layoutTimerId;
+    QKeyEvent* postponed_autorepeat;
 
     int repaintTimerId;
     int scrollTimerId;
@@ -901,57 +904,70 @@ bool KHTMLView::dispatchKeyEvent( QKeyEv
     // The problem here is that Qt generates two autorepeat events \
                (keyrelease+keypress)
     // for autorepeating, while DOM wants only one autorepeat event (keypress), so \
                one
     // of the Qt events shouldn't be passed to DOM, but it should be still filtered
-    // out if DOM would filter the autorepeat event. Therefore, DOM autorepeat \
                events
-    // are sent on Qt autorepeat release (because it comes sooner then press), and \
                the following
-    // Qt autorepeat press is filtered out if the release was filtered out.
-    // Moreover, first Qt (non-autorepeat) keypress should generate DOM keydown \
                followed
-    // by DOM keypress.
+    // out if DOM would filter the autorepeat event. Additional problem is that Qt \
keyrelease +    // events don't have text() set (Qt bug?), so DOM often would ignore \
the keypress event +    // if it was created using Qt keyrelease, but Qt autorepeat \
keyrelease comes +    // before Qt autorepeat keypress (i.e. problem whether to \
filter it out or not). +    // The solution is to filter out and postpone the Qt \
autorepeat keyrelease until +    // the following Qt keypress event comes. If DOM \
accepts the DOM keypress event, +    // the postponed event will be simply discarded. \
If not, it will be passed to keyPressEvent() +    // again, and here it will be \
ignored.  //
     //  Qt:      Press      | Release(autorepeat) Press(autorepeat) etc. |   Release
     //  DOM:   Down + Press |       Press             (nothing)          |     Up
-    //
-    // Additional problem: during autorepeat, the keypress has the text, NOT the \
keyrelease! +    
+    if( _ke == d->postponed_autorepeat ) // replayed event
+        return false;
 
     if( _ke->type() == QEvent::KeyPress )
     {
-        if( _ke->isAutoRepeat())
-        { // ignore autorepeat press and filter out if necessary
-            return d->filter_next_autorepeat_press;
+        if( !_ke->isAutoRepeat())
+        {
+            bool ret = dispatchKeyEventHelper( _ke, false ); // keydown
+            if( dispatchKeyEventHelper( _ke, true )) // keypress
+                ret = true;
+            return ret;
         }
-        bool ret = dispatchKeyEventHelper( _ke, false ); // keydown
-        if( dispatchKeyEventHelper( _ke, true )) // keypress
-            ret = true;
-        return ret;
-    }
-    else // QEvent::KeyRelease
-    {
-        if( _ke->isAutoRepeat())
+        else // autorepeat
         {
             bool ret = dispatchKeyEventHelper( _ke, true ); // keypress
-            d->filter_next_autorepeat_press = ret;
+            if( !ret && d->postponed_autorepeat )
+                keyPressEvent( d->postponed_autorepeat );
+            delete d->postponed_autorepeat;
+            d->postponed_autorepeat = NULL;
             return ret;
         }
-        else
+    }
+    else // QEvent::KeyRelease
+    {
+        if( !_ke->isAutoRepeat())
             return dispatchKeyEventHelper( _ke, false ); // keyup
+        else
+        {
+            Q_ASSERT( d->postponed_autorepeat == NULL );
+            d->postponed_autorepeat = new QKeyEvent( _ke->type(), _ke->key(), \
_ke->ascii(), _ke->state(), +                _ke->text(), _ke->isAutoRepeat(), \
_ke->count()); +            if( _ke->isAccepted())
+                d->postponed_autorepeat->accept();
+            else
+                d->postponed_autorepeat->ignore();
+            return true;
+        }
     }
 }
 
 bool KHTMLView::dispatchKeyEventHelper( QKeyEvent *_ke, bool keypress )
 {
     DOM::NodeImpl* keyNode = m_part->xmlDocImpl()->focusNode();
-    QKeyEvent k(_ke->type(), _ke->key(), _ke->ascii(), _ke->state(),
-                _ke->text(),
-                keypress /*true for autorepeat means keypress, see TextEventImpl*/,
-                _ke->count());
     if (keyNode) {
-        if (keyNode->dispatchKeyEvent(&k)) {
+        if (keyNode->dispatchKeyEvent(_ke, keypress)) {
             return true;
 	}
     }
     else // no focused node, send to document
     {
         // If listener returned false, stop here. Otherwise handle standard keys \
                (#60403).
-        if (!m_part->xmlDocImpl()->dispatchKeyEvent(&k)) {
+        if (!m_part->xmlDocImpl()->dispatchKeyEvent(_ke, keypress)) {
             return true;
         }
     }


["khtml_usage.patch" (text/x-diff)]

--- khtml/html/html_inlineimpl.cpp.sav	2003-10-31 18:21:38.000000000 +0100
+++ khtml/html/html_inlineimpl.cpp	2003-12-04 22:02:37.000000000 +0100
@@ -51,8 +51,7 @@ NodeImpl::Id HTMLAnchorElementImpl::id()
 
 void HTMLAnchorElementImpl::defaultEventHandler(EventImpl *evt)
 {
-    bool keydown = evt->id() == EventImpl::KHTML_KEYPRESS_EVENT ||
-                   evt->id() == EventImpl::KHTML_KEYDOWN_EVENT;
+    bool keydown = evt->id() == EventImpl::KHTML_KEYDOWN_EVENT;
 
     // React on clicks and on keypresses.
     // Don't make this KEYUP_EVENT again, it makes khtml follow links
--- khtml/html/html_formimpl.cpp.sav	2003-12-02 15:11:53.000000000 +0100
+++ khtml/html/html_formimpl.cpp	2003-12-04 22:09:47.000000000 +0100
@@ -804,7 +804,7 @@ void HTMLGenericFormElementImpl::default
             }
         }
 
-	if (evt->id() == EventImpl::KHTML_KEYDOWN_EVENT) {
+	if (evt->id() == EventImpl::KHTML_KEYPRESS_EVENT) {
 	    TextEventImpl * k = static_cast<TextEventImpl *>(evt);
 	    int key = k->qKeyEvent ? k->qKeyEvent->key() : 0;
 	    if (m_render && (key == Qt::Key_Tab || key == Qt::Key_BackTab)) {


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

Configure | About | News | Add a list | Sponsored by KoreLogic