Standalone MediaDocuments do not play/pause when you press space bar. <rdar://problem/6635099>
Created attachment 29102 [details] patch Needs manual testing since it is a standalone media document.
Comment on attachment 29102 [details] patch > diff --git a/WebCore/loader/MediaDocument.cpp b/WebCore/loader/MediaDocument.cpp > index b89ac10..85aa48d 100644 > + if (event->type() == eventNames().keydownEvent && event->isKeyboardEvent()) { > + KeyboardEvent* k = static_cast<KeyboardEvent*>(event); Please use a better variable name than 'k'. > + HTMLVideoElement* video; > + if (targetNode) { > + if (targetNode->hasTagName(videoTag)) > + video = static_cast<HTMLVideoElement*>(targetNode); > + else { > + RefPtr<NodeList> nodeList = targetNode->getElementsByTagName("video"); > + if (nodeList.get()->length() > 0) > + video = static_cast<HTMLVideoElement*>(nodeList.get()->item(0)); When does this happen? When the event target is the document? Since this is MediaDocument.cpp, can you get to the media element directly? > + } > + } What about standalone audio? > + if (video) { > + if (k->keyIdentifier() == "U+0020") { // space This is convention used elsewhere? r- for now, but I think it's generally sound.
> When does this happen? When the event target is the document? Since this is MediaDocument.cpp, > can you get to the media element directly? Yes, when the target is the document. The alternative would be to have the user focus the media element and then wait for the keyboard event, which is obviously not useful (and a regression). And no, you can't get to the media element directly. The MediaDocument fires up a tokeniser which inserts the video directly. I could work it so the document holds a reference to the media - I wasn't sure which was the best way. > What about standalone audio? Standalone audio is standalone video. The MediaDocument tokeniser always makes a video element. > This is convention used elsewhere? I had a choice here between looking at the keyCode (would be 32 for space) or the keyIdentifier. Most of the other code I looked at uses keyIdentifier. I'm not sure why, since it is string comparison.
Looking again, it seems sometimes people use keyCode and sometimes they use keyIdentifier. Obviously they use keyIdentifier for things like PageUp and Enter, but see HTMLButtonElement for an example checking for U+0020 like I do. I don't care either way.
Keeping a reference to the media element would be a more intrusive change (eg. changing two class definitions rather than just this method). However, since we're in a standalone document it's only one extra reference - not a memory issue. Upside is that we don't search the document with every keypress. Either way fine with me, but I'm tempted to keep the changes to a minimum.
Of course, I meant ie not eg. See http://www.imdb.com/title/tt0113161/quotes
Comment on attachment 29102 [details] patch OK, r+ but please use a better name than 'k' (suggest: "keyboardEvent").
Committed r42134 M WebCore/ChangeLog M WebCore/loader/MediaDocument.cpp