--Boundary-00=_AXJUATC2qx+jV/7
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
On Monday 26 of January 2004 06:05, Dirk Mueller wrote:
> On Friday 23 January 2004 17:04, Lubos Lunak wrote:
So, now it should be right hopefully. I've split the changes to 3 patches, so
that the changes I'm not quite sure about are independent from the main
accesskey code.
The main code is in khtml.patch. It's fairly simple, every accesskey is
translated to click() - Mozilla does it the same way, and this ensures that
onclick handlers are called. In fact, the old event handling translated some
of the keyevents to clicks too.
The status of various elements:
- BUTTON doesn't work - I noticed its support is quite bad in general in KHTML
- LABEL and LEGEND don't work
- TEXTAREA works, but accesskeys don't work when it's focused
- INPUT - only buttons (including submit/reset) and lineedits work
- A, AREA work
- there's code in KHTML that suggests support for accesskey in SELECT was
planned too (HTMLSelectElementImpl::parseAttribute()), but I don't see it in
HTML docs, even though it makes sense !?
> > > Furthermore, setFocusNode can trigger something that makes
> > > NodeImpl* node point to garbage, you don't want that to happen. To fix,
> > > make a Node guard, like
> > >
> > > Node focusguard(node);
> > > m_part->xmlDocImpl()->setFocusNode(node);
> > > ...
> > > emit m_part->nodeActivated(focusguard);
> > So in other words, KHTMLView::focusNextPrevNode() has this broken as
> > well.
>
> Yes.
khtmlview.cpp.patch . I hope it's right.
These two patches are IMHO safe enough to be backported.
>
> > Is it documented somewhere how this should work? The best docs I could
> > find only explained what the event is, but not when it's generated and so
> > on. I e.g. noticed that if I do , clicking or pressing Space
> > on the button shows the alert, by my implementation of accesskey doesn't
> > trigger it (it's triggered in Mozilla).
>
> Well, normally when you click on an element, and it receives the event
> during the capturing phase, it will decide if it can handle the event. that
> means, the element can handle it, is not disabled or anything that would
> prevent event handling.
>
> When the click activates the element, a domactivate event is emitted. event
> listeners can react to that event, and can even block it. if the element
> receives its domactivate event, it does its default action (in case of a
> link, go to the url).
>
> so its a slight mistake for an anchor to react on a click event. instead it
> should produce a activation event, and then react on that one. Feel free to
> fix.
html_formimpl.cpp.patch - INPUT elements perform the action in activate(),
key events are transformed to clicks, clicks are transformed to domactivate
events. I hope I got it right, because it's mostly reshuffling of the stuff,
but I'm far from being 100% sure about it.
--
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/
--Boundary-00=_AXJUATC2qx+jV/7
Content-Type: text/x-diff;
charset="iso-8859-1";
name="khtml.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="khtml.patch"
--- khtml/xml/dom_docimpl.cpp.sav 2004-01-23 17:24:42.000000000 +0100
+++ khtml/xml/dom_docimpl.cpp 2004-02-05 23:24:04.000000000 +0100
@@ -1431,6 +1431,29 @@ NodeImpl *DocumentImpl::previousFocusNod
}
}
+ElementImpl* DocumentImpl::findAccessKeyElement(QChar c)
+{
+ c = c.upper();
+ for( NodeImpl* n = this;
+ n != NULL;
+ n = n->traverseNextNode()) {
+ if( n->id() == ID_A
+ || n->id() == ID_AREA
+ || n->id() == ID_BUTTON
+ || n->id() == ID_LABEL
+ || n->id() == ID_TEXTAREA
+ || n->id() == ID_LEGEND
+ || n->id() == ID_INPUT ) {
+ ElementImpl* en = static_cast< ElementImpl* >( n );
+ DOMString s = en->getAttribute( ATTR_ACCESSKEY );
+ if( s.length() == 1
+ && s[ 0 ].upper() == c )
+ return en;
+ }
+ }
+ return NULL;
+}
+
int DocumentImpl::nodeAbsIndex(NodeImpl *node)
{
assert(node->getDocument() == this);
--- khtml/xml/dom_docimpl.h.sav 2004-01-23 17:24:42.000000000 +0100
+++ khtml/xml/dom_docimpl.h 2001-01-01 01:01:00.000000000 +0100
@@ -404,6 +404,8 @@ public:
*/
NodeImpl *previousFocusNode(NodeImpl *fromNode);
+ ElementImpl* findAccessKeyElement(QChar c);
+
int nodeAbsIndex(NodeImpl *node);
NodeImpl *nodeWithAbsIndex(int absIndex);
--- khtml/html/html_formimpl.cpp.sav 2004-02-06 00:05:09.000000000 +0100
+++ khtml/html/html_formimpl.cpp 2004-02-06 00:15:47.000000000 +0100
@@ -951,6 +951,12 @@ void HTMLButtonElementImpl::activate()
m_form->reset();
}
+void HTMLButtonElementImpl::click()
+{
+ QMouseEvent me(QEvent::MouseButtonRelease, QPoint(0,0),Qt::LeftButton, 0);
+ dispatchMouseEvent(&me,EventImpl::CLICK_EVENT, 1);
+}
+
bool HTMLButtonElementImpl::encoding(const QTextCodec* codec, khtml::encodingList& encoding, bool /*multipart*/)
{
if (m_type != SUBMIT || name().isEmpty() || !m_activeSubmit)
@@ -1157,9 +1163,10 @@ void HTMLInputElementImpl::parseAttribut
if (m_render && m_type == IMAGE) m_render->updateFromElement();
break;
case ATTR_USEMAP:
- case ATTR_ACCESSKEY:
// ### ignore for the moment
break;
+ case ATTR_ACCESSKEY:
+ break;
case ATTR_WIDTH:
// ignore this attribute, do _not_ add
// a CSS_PROP_WIDTH here!
@@ -1586,7 +1593,6 @@ void HTMLLegendElementImpl::parseAttribu
switch(attr->id())
{
case ATTR_ACCESSKEY:
- // ### ignore for the moment
break;
default:
HTMLElementImpl::parseAttribute(attr);
@@ -1808,7 +1814,6 @@ void HTMLSelectElementImpl::parseAttribu
m_multiple = (attr->val() != 0);
break;
case ATTR_ACCESSKEY:
- // ### ignore for the moment
break;
case ATTR_ONCHANGE:
setHTMLEventListener(EventImpl::CHANGE_EVENT,
@@ -2215,7 +2220,6 @@ void HTMLTextAreaElementImpl::parseAttri
m_wrap = ta_NoWrap;
break;
case ATTR_ACCESSKEY:
- // ignore for the moment
break;
case ATTR_ONSELECT:
setHTMLEventListener(EventImpl::SELECT_EVENT,
--- khtml/html/html_inlineimpl.cpp.sav 2004-01-30 16:05:01.000000000 +0100
+++ khtml/html/html_inlineimpl.cpp 2004-02-05 02:41:15.000000000 +0100
@@ -160,6 +160,12 @@ void HTMLAnchorElementImpl::defaultEvent
}
+void HTMLAnchorElementImpl::click()
+{
+ QMouseEvent me(QEvent::MouseButtonRelease, QPoint(0,0),Qt::LeftButton, 0);
+ dispatchMouseEvent(&me,EventImpl::CLICK_EVENT, 1);
+}
+
void HTMLAnchorElementImpl::parseAttribute(AttributeImpl *attr)
{
switch(attr->id())
@@ -174,6 +180,8 @@ void HTMLAnchorElementImpl::parseAttribu
case ATTR_TITLE:
case ATTR_REL:
break;
+ case ATTR_ACCESSKEY:
+ break;
default:
HTMLElementImpl::parseAttribute(attr);
}
--- khtml/html/html_inlineimpl.h.sav 2003-02-11 10:42:08.000000000 +0100
+++ khtml/html/html_inlineimpl.h 2001-01-01 01:01:00.000000000 +0100
@@ -41,6 +41,7 @@ public:
virtual Id id() const;
virtual void parseAttribute(AttributeImpl *attr);
virtual void defaultEventHandler(EventImpl *evt);
+ void click();
protected:
bool m_hasTarget : 1;
};
--- khtml/html/html_formimpl.h.sav 2004-02-05 23:34:16.000000000 +0100
+++ khtml/html/html_formimpl.h 2001-01-01 01:01:00.000000000 +0100
@@ -195,6 +195,7 @@ public:
virtual bool encoding(const QTextCodec*, khtml::encodingList&, bool);
void activate();
virtual void attach();
+ void click();
protected:
DOMString m_value;
--- khtml/khtmlview.h.sav 2003-12-11 12:51:24.000000000 +0100
+++ khtml/khtmlview.h 2001-01-01 01:01:00.000000000 +0100
@@ -239,6 +239,8 @@ private:
bool scrollTo(const QRect &);
void focusNextPrevNode(bool next);
+ bool handleAccessKey(const QKeyEvent* ev);
+ bool focusNodeWithAccessKey(QChar c, KHTMLView* caller = NULL);
void useSlowRepaints();
--- khtml/khtmlview.cpp.sav 2004-01-30 16:04:59.000000000 +0100
+++ khtml/khtmlview.cpp 2004-02-06 00:12:24.000000000 +0100
@@ -32,6 +32,7 @@
#include "html/html_documentimpl.h"
#include "html/html_inlineimpl.h"
+#include "html/html_formimpl.h"
#include "rendering/render_arena.h"
#include "rendering/render_canvas.h"
#include "rendering/render_frames.h"
@@ -1028,6 +1029,13 @@ void KHTMLView::keyPressEvent( QKeyEvent
}
#endif // KHTML_NO_CARET
+ // accesskey handling needs to be done before dispatching, otherwise e.g. lineedits
+ // may eat the event
+ if( handleAccessKey( _ke )) {
+ _ke->accept();
+ return;
+ }
+
if ( dispatchKeyEvent( _ke )) {
// If either keydown or keypress was accepted by a widget, or canceled by JS, stop here.
_ke->accept();
@@ -1593,6 +1601,100 @@ void KHTMLView::focusNextPrevNode(bool n
emit m_part->nodeActivated(Node(newFocusNode));
}
+// Handling of the HTML accesskey attribute.
+bool KHTMLView::handleAccessKey( const QKeyEvent* ev )
+{
+ const int mods = Qt::AltButton | Qt::ControlButton;
+ if( ( ev->state() & mods ) != mods )
+ return false;
+// Qt interprets the keyevent also with the modifiers, and ev->text() matches that,
+// but this code must act as if the modifiers weren't pressed
+ QChar c;
+ if( ev->key() >= Key_A && ev->key() <= Key_Z )
+ c = 'A' + ev->key() - Key_A;
+ else if( ev->key() >= Key_0 && ev->key() <= Key_9 )
+ c = '0' + ev->key() - Key_0;
+ else {
+ // TODO fake XKeyEvent and XLookupString ?
+ // This below seems to work e.g. for eacute though.
+ if( ev->text().length() == 1 )
+ c = ev->text()[ 0 ];
+ }
+ if( c.isNull())
+ return false;
+ return focusNodeWithAccessKey( c );
+}
+
+bool KHTMLView::focusNodeWithAccessKey( QChar c, KHTMLView* caller )
+{
+ DocumentImpl *doc = m_part->xmlDocImpl();
+ if( !doc )
+ return false;
+ ElementImpl* node = doc->findAccessKeyElement( c );
+ if( !node ) {
+ QPtrList frames = m_part->frames();
+ for( QPtrListIterator it( frames );
+ it != NULL;
+ ++it ) {
+ if( !(*it)->inherits( "KHTMLPart" ))
+ continue;
+ KHTMLPart* part = static_cast< KHTMLPart* >( *it );
+ if( part->view() && part->view() != caller
+ && part->view()->focusNodeWithAccessKey( c, this ))
+ return true;
+ }
+ // pass up to the parent
+ if (m_part->parentPart() && m_part->parentPart()->view()
+ && m_part->parentPart()->view() != caller )
+ return m_part->parentPart()->view()->focusNodeWithAccessKey( c, this );
+ return false;
+ }
+
+ // Scroll the view as necessary to ensure that the new focus node is visible
+#ifndef KHTML_NO_CARET
+ // if it's an editable element, activate the caret
+ if (!m_part->isCaretMode() && !m_part->isEditable()
+ && node->contentEditable()) {
+ d->caretViewContext();
+ moveCaretTo(node, 0L, true);
+ } else {
+ caretOff();
+ }
+#endif // KHTML_NO_CARET
+
+ if (!scrollTo(node->getRect()))
+ return true;
+
+ if( node->isSelectable()) {
+ // Set focus node on the document
+ m_part->xmlDocImpl()->setFocusNode(node);
+ emit m_part->nodeActivated(Node(node));
+ }
+ switch( node->id()) {
+ case ID_A:
+ static_cast< HTMLAnchorElementImpl* >( node )->click();
+ break;
+ case ID_INPUT:
+ static_cast< HTMLInputElementImpl* >( node )->click();
+ break;
+ case ID_BUTTON:
+ static_cast< HTMLButtonElementImpl* >( node )->click();
+ break;
+ case ID_AREA:
+ static_cast< HTMLAreaElementImpl* >( node )->click();
+ break;
+ case ID_LABEL:
+ // TODO should be click(), after it works for label
+ break;
+ case ID_TEXTAREA:
+ break; // just focusing it is enough
+ case ID_LEGEND:
+ // TODO
+ break;
+ }
+ return true;
+}
+
void KHTMLView::setMediaType( const QString &medium )
{
m_medium = medium;
--Boundary-00=_AXJUATC2qx+jV/7
Content-Type: text/x-diff; charset="iso-8859-1"; name="khtmlview.cpp.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="khtmlview.cpp.patch"
--- khtmlview.cpp.sav 2004-03-10 00:29:46.000000000 +0100
+++ khtmlview.cpp 2004-03-10 00:30:19.000000000 +0100
@@ -1587,7 +1587,10 @@ void KHTMLView::focusNextPrevNode(bool n
}
// Set focus node on the document
+ Node guard(newFocusNode);
m_part->xmlDocImpl()->setFocusNode(newFocusNode);
+ if( newFocusNode != NULL && newFocusNode->hasOneRef()) // deleted, only held by guard
+ return;
emit m_part->nodeActivated(Node(newFocusNode));
}
@@ -1655,10 +1658,15 @@ bool KHTMLView::focusNodeWithAccessKey(
if (!scrollTo(node->getRect()))
return true;
+ Node guard( node );
if( node->isSelectable()) {
// Set focus node on the document
m_part->xmlDocImpl()->setFocusNode(node);
+ if( node != NULL && node->hasOneRef()) // deleted, only held by guard
+ return true;
emit m_part->nodeActivated(Node(node));
+ if( node != NULL && node->hasOneRef())
+ return true;
}
switch( node->id()) {
case ID_A:
--Boundary-00=_AXJUATC2qx+jV/7
Content-Type: text/x-diff; charset="iso-8859-1"; name="html_formimpl.cpp.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="html_formimpl.cpp.patch"
--- html_formimpl.cpp.sav 2004-03-11 14:37:46.000000000 +0100
+++ html_formimpl.cpp 2004-03-11 17:13:49.000000000 +0100
@@ -1457,11 +1457,6 @@ void HTMLInputElementImpl::defaultEventH
if (evt->isMouseEvent()) {
MouseEventImpl *me = static_cast(evt);
- if ((m_type == RADIO || m_type == CHECKBOX)
- && me->id() == EventImpl::MOUSEUP_EVENT && me->detail() > 0) {
- // click will follow
- setChecked(m_type == RADIO ? true : !checked());
- }
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;
@@ -1471,49 +1466,50 @@ void HTMLInputElementImpl::defaultEventH
}
}
- if (m_type == RADIO || m_type == CHECKBOX || m_type == SUBMIT || m_type == RESET || m_type == BUTTON ) {
- bool check = false;
- if (active() && ( evt->id() == EventImpl::KEYUP_EVENT ||
- evt->id() == EventImpl::KHTML_KEYPRESS_EVENT ) ) {
- TextEventImpl *te = static_cast(evt);
- if (te->keyVal() == ' ')
- check = true;
- }
- if (check) {
- if (evt->id() == EventImpl::KEYUP_EVENT) {
- if (m_type == RADIO || m_type == CHECKBOX)
- setChecked(m_type == RADIO ? true : !checked());
- click();
- }
- // Tell the parent that we handle this key (keyup and keydown), even though only keyup activates (#70478)
- evt->setDefaultHandled();
- }
+ if( evt->isTextEvent()) {
+ if (m_type == RADIO || m_type == CHECKBOX || m_type == SUBMIT
+ || m_type == RESET || m_type == BUTTON || m_type == IMAGE) {
+ TextEventImpl *te = static_cast(evt);
+ bool clck = false;
+ if (te->keyVal() == ' ')
+ clck = true;
+ if( m_type == SUBMIT || m_type == RESET || m_type == IMAGE ) {
+ QKeyEvent *ke = static_cast(evt)->qKeyEvent;
+ if (ke && active() && (ke->key() == Qt::Key_Return
+ || ke->key() == Qt::Key_Enter || ke->key() == Qt::Key_Space))
+ clck = true;
+ }
+ if( clck ) {
+ if( evt->id() == EventImpl::KEYUP_EVENT )
+ click();
+ // Tell the parent that we handle this key (keyup and keydown), even though only keyup activates (#70478)
+ evt->setDefaultHandled();
+ }
+ }
}
-
// DOMActivate events cause the input to be "activated" - in the case of image and submit inputs, this means
// actually submitting the form. For reset inputs, the form is reset. These events are sent when the user clicks
// on the element, or presses enter while it is the active element. Javascript code wishing to activate the element
// must dispatch a DOMActivate event - a click event will not do the job.
- if (m_type == IMAGE || m_type == SUBMIT || m_type == RESET) {
- bool act = (evt->id() == EventImpl::DOMACTIVATE_EVENT);
- if (!act && evt->id() == EventImpl::KEYUP_EVENT) {
- QKeyEvent *ke = static_cast(evt)->qKeyEvent;
- if (ke && active() && (ke->key() == Qt::Key_Return || ke->key() == Qt::Key_Enter || ke->key() == Qt::Key_Space))
- act = true;
- }
- if (act)
- activate();
- }
+ if( evt->id() == EventImpl::DOMACTIVATE_EVENT )
+ activate();
}
HTMLGenericFormElementImpl::defaultEventHandler(evt);
}
void HTMLInputElementImpl::activate()
{
- if (!m_form || !m_render)
+ if (m_type == RADIO || m_type == CHECKBOX) {
+ setChecked(m_type == RADIO ? true : !checked());
+ return;
+ } else if( m_type == TEXT || m_type == PASSWORD || m_type == FILE
+ || m_type == HIDDEN || m_type == BUTTON ) {
return;
+ }
+ if (!m_form || !m_render)
+ return;
m_clicked = true;
if (m_type == RESET) {
m_form->reset();
--Boundary-00=_AXJUATC2qx+jV/7--