Bug 51591 – [GTK] Disable flash plugin in GtkLauncher when using gtk3

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Garcia Campos]

Flash plugin uses gtk2 that is incompatible with gtk3, so when webkit is built with gtk3, flash plugin makes us crash.

[Carlos Garcia Campos]

Created attachment 77414 [details]
Patch to disable flash plugin in GtkLauncher when using gtk3

[Eric Seidel (no email)]

Comment on attachment 77414 [details]
Patch to disable flash plugin in GtkLauncher when using gtk3

Shouldn't this be in a separate helper function (static) for better code encapsulation?

[Daniel Bates]

Comment on attachment 77414 [details]
Patch to disable flash plugin in GtkLauncher when using gtk3

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

> Tools/GtkLauncher/main.c:247
> +    for (l = plugins; l; l = g_slist_next(l)) {

Nit: I suggest renaming 'l' (lowercase 'L' character) to some other variable name (maybe p?) because it looks similar to 'I' (capital eye character) for many fonts, including Courier, Helvetica and Lucida Grande. Moreover, 'p' is closer to the word plugin than 'l'.

Additionally, I suggest inlining the declaration of 'l' in the for-loop initialization clause since its scope is only relevant within the loop body.

[Martin Robinson]

Comment on attachment 77414 [details]
Patch to disable flash plugin in GtkLauncher when using gtk3

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

>> Tools/GtkLauncher/main.c:247
>> +    for (l = plugins; l; l = g_slist_next(l)) {
> 
> Nit: I suggest renaming 'l' (lowercase 'L' character) to some other variable name (maybe p?) because it looks similar to 'I' (capital eye character) for many fonts, including Courier, Helvetica and Lucida Grande. Moreover, 'p' is closer to the word plugin than 'l'.
> 
> Additionally, I suggest inlining the declaration of 'l' in the for-loop initialization clause since its scope is only relevant within the loop body.

Moving the declaration of 'l' may produce a warning since this file is compiled with -ansi. If that's the case, I'm in favor of dropping -ansi. We should be using c99 at this point.

[Carlos Garcia Campos]

Created attachment 77489 [details]
Updated patch

Thanks for the comments, this is an updated patch that moves the code to a function and renames the list iterator to 'p'. I haven't moved the list declaration to the for initialization clause because we are building with -ansi flag.

[WebKit Commit Bot]

Comment on attachment 77489 [details]
Updated patch

Clearing flags on attachment: 77489

Committed r74685: <http://trac.webkit.org/changeset/74685>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/74685 might have broken Leopard Intel Debug (Tests)