[Webkit-unassigned] [Bug 94373] [gtk] adding methods and signals for usermedia communication between webkit and browser
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Oct 16 10:41:19 PDT 2012
https://bugs.webkit.org/show_bug.cgi?id=94373
--- Comment #34 from Gustavo Noronha (kov) <gns at gnome.org> 2012-10-16 10:42:09 PST ---
(From update of attachment 168593)
View in context: https://bugs.webkit.org/attachment.cgi?id=168593&action=review
The API looks OK to me in general, but I'll try to review the spec later today and see if I can spot any problems.
> Source/WebKit/gtk/ChangeLog:6
> + When the user make a getUserMedia call, webview will emit a sigial (user-media-requested)
make -> makes sigial -> signal
> Source/WebKit/gtk/ChangeLog:7
> + to the application containing then request itself (webkitwebusermediarequest) and two
then -> the
> Source/WebKit/gtk/ChangeLog:8
> + list with the available audio and video devices (webkitwebusermedialist).
list -> lists
> Source/WebKit/gtk/ChangeLog:17
> + On the case the user reject the request, webkit_web_user_media_request_fail must
On the case -> In case?
> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:25
> +
No blank line
> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:39
> + GtkWidget* vbox;
> + GtkWidget* audioMessage = 0;
> + GtkWidget* videoMessage = 0;
How about making this a GtkBuilder file that can more easily be changed if necessary?
> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:43
> + gint audioListLength = audioSource.size();
> + gint videoListLength = videoSource.size();
We tend not to use glib types for types that are simple C types.
> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:72
> + audioMessage = gtk_label_new(hasAudio ? _("Select from available user audio") : _("No user audio available"));
These messages are quite cryptic for someone that doesn't know the spec I think. User audios are audio inputs? 'audio' should use the plural in this case I imagine?
> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:88
> + gtk_combo_box_text_append_text(GTK_COMBO_BOX_TEXT(m_audioCombo), g_strdup(audioSource[i]->name().utf8().data()));
You're leaking the string that you create with g_strdup. Why g_strdup, if the function takes a const char*?
> Source/WebKit/gtk/webkit/webkitwebusermediarequest.cpp:131
> + * webkit_web_user_media_request_succeed:
How about sticking to the allow/deny terminology we use for our other policy decisions?
--
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