Bug 51504 – Make Speech menu items work in WebKit2

• Component: WebKit2
• Status: RESOLVED FIXED

[Darin Adler]

Make Speech menu items work in WebKit2

[Darin Adler]

Created attachment 77272 [details]
Patch

[Darin Adler]

<rdar://problem/8246002>

[Early Warning System Bot]

Attachment 77272 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7287108

[Anders Carlsson]

Comment on attachment 77272 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77272&action=review

> WebKit2/UIProcess/API/mac/WKView.mm:350
> +        return YES;

This should return "enabled".

[Darin Adler]

Committed r74521: <http://trac.webkit.org/changeset/74521>

[Darin Adler]

Committed r74522: <http://trac.webkit.org/changeset/74522>

[WebKit Review Bot]

http://trac.webkit.org/changeset/74521 might have broken SnowLeopard Intel Release (Tests)
The following tests are not passing:
http/tests/local/stylesheet-and-script-load-order-media-print.html

[Maciej Stachowiak]

Comment on attachment 77272 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77272&action=review

> WebKit2/UIProcess/WebPageProxy.messages.in:119
> +    # Keyboard support messages.

This and other changes seem to add periods to the end of sentence fragments, which seems wrong.

[Maciej Stachowiak]

Comment on attachment 77272 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77272&action=review

> WebKit2/WebProcess/WebPage/WebPage.cpp:1377
> +bool WebPage::isSpeaking()
> +{
> +    bool result;
> +    return sendSync(Messages::WebPageProxy::GetIsSpeaking(), Messages::WebPageProxy::GetIsSpeaking::Reply(result)) && result;
> +}

It seems suboptimal to do synchronous messaging to the UI process just to validate this one context menu item. Is there a way we can have the validation happen in the UI process in the first place? That seems like the right way to do it, since that's where the action will happen. In fact, the popup menu runs in the UI process, it would be better if WebKit-provided items could be handled in the UI process when appropriate.

[Darin Adler]

Comment on attachment 77272 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77272&action=review

>> WebKit2/UIProcess/WebPageProxy.messages.in:119
>> +    # Keyboard support messages.
> 
> This and other changes seem to add periods to the end of sentence fragments, which seems wrong.

These periods were on all the other sentence fragments in the file; I corrected the few that were different to be consistent. It’s OK if you want to be consistent the other way.

>> WebKit2/WebProcess/WebPage/WebPage.cpp:1377
>> +}
> 
> It seems suboptimal to do synchronous messaging to the UI process just to validate this one context menu item. Is there a way we can have the validation happen in the UI process in the first place? That seems like the right way to do it, since that's where the action will happen. In fact, the popup menu runs in the UI process, it would be better if WebKit-provided items could be handled in the UI process when appropriate.

Yes, that would be a nice refinement to make later. This works well, but what you describe would be even better.