[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