Bug 46873

Thanks for pointing this out, Maciej!

http://dev.w3.org/html5/spec/Overview.html#extensibility I don't see anything about event handlers, so I assume what we have is OK?

I believe Jeremy is asking about the planned addition of a new event 'onspeechchange', this is not implemented yet. From the spec above I wonder if the event handler is treated the same way attributes are, so the event handler would become x-webkit-onspeechchange in the markup and webkitonspeechchange - though it doesn't seem right for an event handler to not have the 'on' prefix.

Looking at existing features, I see the patterns such as 'onwebkitfullscreenchange' and the same name is used both in the markup as the attribute name and in the IDL. So looks like for speech input we can have the new event handler named as 'onwebkitspeechchange' for both the markup and IDL. Maciej, can you please confirm if my understanding is correct?

I think if the event name is prefixed, then t's probably better to use the normal onfoo attributes. The event name itself should not contain "on", that's only for the attribute. So I'd suggest: event name: webkitspeechchange event handler attribute: onwebkitspeechchange Maybe it would be good for HTML5 to suggest that for extended event handler attributes too.

Looks like if I add x-webkit-speech to HTMLAttributeNames.in, the bindings code generator emits the c++ code for attribute named "x_webkit_speechAttr". But adding an IDL attribute named 'webkitSpeech' expects the attribute to be named as 'webkitSpeechAttr', which doesn't match the above generated code. Perhaps this requires an update to the bindings generator? I'm looking at that now.. please let me know if this is not the right track.

Created attachment 69334 [details] Patch

I've uploaded a patch to rename the attribute, it required some additions to the dom/make_names.pl script as well. Please take a look and suggest if any other webkit reviewer should have a look as well.

Comment on attachment 69334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69334&action=review > WebCore/dom/make_names.pl:187 > + if ($attr =~ m/^x-webkit/) { Maybe do m/^x-webkit-(.*)/ and then use the $1 as the attribute? > WebCore/dom/make_names.pl:193 > + $attr =~ s/-/_/g; You probably want this in both code paths, right?

Created attachment 69646 [details] Patch Addressed both comments, please take another look.

seems fine to me

Comment on attachment 69646 [details] Patch Rejecting patch 69646 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']" exit_code: 2 Last 500 characters of output: ful. Files=14, Tests=304, 1 wallclock secs ( 0.63 cusr + 0.13 csys = 0.76 CPU) Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Users/abarth/git/webkit-queue/LayoutTests Testing 21454 test cases. http/tests/xmlhttprequest/basic-auth.html -> failed Exiting early after 1 failures. 21300 tests run. 532.30s total testing time 21299 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 34 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/4142077

Comment on attachment 69646 [details] Patch The test failure looks unrelated to this change, so trying the commit queue one more time..

Comment on attachment 69646 [details] Patch Clearing flags on attachment: 69646 Committed r69099: <http://trac.webkit.org/changeset/69099>

All reviewed patches have been landed. Closing bug.