[PATCH] accesskey attribute (#45788)

[Dirk Mueller]

On Thursday 22 January 2004 13:44, Lubos Lunak wrote:

>  please review the attached khtml.patch for adding support for the
> accesskey attribute. I based the searching and focusing code on the one
> used for tabbing to next element, so I hope I got it right.

the html_inlineimpl patch is okay. The other patch has a few faults: 

> -    case ATTR_ACCESSKEY:
>        // ### ignore for the moment
>         break;
>+    case ATTR_ACCESSKEY:
>+        break;

That code is entirely redundant. with your implementation, all those case 
statements you added (and which were already there) can be removed. I planned 
to add a "registerAccessKey" method to DocumentImpl thats why those place 
holders where there. 

+    // Set focus node on the document
+    m_part->xmlDocImpl()->setFocusNode(node);
+    emit m_part->nodeActivated(Node(node));
+    node->dispatchUIEvent(EventImpl::DOMACTIVATE_EVENT, 1);

I think the activation should be done before you emit the activate_d_ signal. 
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);

>  The second, more important problem I have is how to actually trigger the
> action in the HTML element.
> http://lists.w3.org/Archives/Public/www-dom/2002AprJun/0137.html makes me
> believe just dispatching DOMActivate event like the patch currently does
> should do, but it doesn't really work for most elements. As I lack any
> serious knowledge of KHTML/DOM, I have no idea if this is KHTML bug or if
> it should be done differently.

Its mostly a KHTML bug, or rather, a missing feature. 

>  PS: Who can I annoy about adding accesskey's for next/previous at
> bugs.kde.org ;) ?

cvs co bugs/template


Dirk