[Webkit-unassigned] [Bug 106539] PDFPlugin: Tab between text annotations
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Feb 25 10:00:52 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=106539
Darin Adler <darin at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #186973|review? |review+
Flag| |
--- Comment #7 from Darin Adler <darin at apple.com> 2013-02-25 10:03:16 PST ---
(From update of attachment 186973)
View in context: https://bugs.webkit.org/attachment.cgi?id=186973&action=review
review+ but only if you add a check of isKeyboardEvent to fix the bad cast
> Source/WebCore/ChangeLog:9
> + * WebCore.exp.in: Export KeyboardEvent::getModifierState.
Why? I don’t see it being used in this patch. Is it used indirectly?
> Source/WebKit2/WebProcess/Plugins/PDF/PDFPluginTextAnnotation.h:63
> + virtual bool operator==(const EventListener& listener) OVERRIDE { return this == &listener; }
Normally a virtual == operator is an anti-pattern. Not sure if there’s any real problem with it here.
> Source/WebKit2/WebProcess/Plugins/PDF/PDFPluginTextAnnotation.mm:131
> +void PDFPluginTextAnnotation::PDFPluginTextAnnotationEventListener::handleEvent(ScriptExecutionContext*, Event* event)
The code in EventHandler::defaultTabEventHandler checks Page::tabKeyCyclesThroughElements. Is that something we need to do too?
> Source/WebKit2/WebProcess/Plugins/PDF/PDFPluginTextAnnotation.mm:134
> + if (event->type() == eventNames().keydownEvent) {
> + KeyboardEvent* keyboardEvent = static_cast<KeyboardEvent*>(event);
You need to check the type of the event before casting to KeyboardEvent by calling isKeyboardEvent. A script can create an event with a given event name that is not a “real” event of that type and if so this will be a bad cast.
> Source/WebKit2/WebProcess/Plugins/PDF/PDFPluginTextAnnotation.mm:136
> + if (keyboardEvent->keyCode() == VK_TAB) {
The code in EventHandler::defaultKeyboardEventHandler uses keyIdentifier rather than keyCode. Any reason you chose differently?
> Source/WebKit2/WebProcess/Plugins/PDF/PDFPluginTextAnnotation.mm:138
> + if (keyboardEvent->ctrlKey() || keyboardEvent->metaKey() || keyboardEvent->altKey() || keyboardEvent->altGraphKey())
> + return;
The code in EventHandler::defaultTabEventHandler does not check altKey. Any reason for the difference?
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list