WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
51504
Make Speech menu items work in WebKit2
https://bugs.webkit.org/show_bug.cgi?id=51504
Summary
Make Speech menu items work in WebKit2
Darin Adler
Reported
2010-12-22 16:00:21 PST
Make Speech menu items work in WebKit2
Attachments
Patch
(39.55 KB, patch)
2010-12-22 16:13 PST
,
Darin Adler
andersca
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Darin Adler
Comment 1
2010-12-22 16:13:03 PST
Created
attachment 77272
[details]
Patch
Darin Adler
Comment 2
2010-12-22 16:14:42 PST
<
rdar://problem/8246002
>
Early Warning System Bot
Comment 3
2010-12-22 16:27:59 PST
Attachment 77272
[details]
did not build on qt: Build output:
http://queues.webkit.org/results/7287108
Anders Carlsson
Comment 4
2010-12-22 16:31:59 PST
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
Comment 5
2010-12-22 16:45:19 PST
Committed
r74521
: <
http://trac.webkit.org/changeset/74521
>
Darin Adler
Comment 6
2010-12-22 16:50:29 PST
Committed
r74522
: <
http://trac.webkit.org/changeset/74522
>
WebKit Review Bot
Comment 7
2010-12-22 17:19:21 PST
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 8
2010-12-22 20:25:18 PST
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 9
2010-12-22 20:27:48 PST
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 10
2010-12-23 08:28:21 PST
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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug