Bug 35155 – [Qt] Allow QtLauncher to be started without Plugin support

• Status: RESOLVED
• Resolution: INVALID
• Component: Tools / Tests

[Casper van Donderen]

Created attachment 49073 [details]
Patch to start without plugins with -no-plugins switch

Add a -no-plugins switch to start the QtLauncher without plugins.

The attached patch does not interfere with applyDefaultSettings(); since there are no settings that will be overwritten.

[Casper van Donderen]

Comment on attachment 49073 [details]
Patch to start without plugins with -no-plugins switch

Forgot to add the new flag in the usage string

[Casper van Donderen]

Created attachment 49077 [details]
Patch to allow loading the QtLauncher with a -no-plugins option

[Eric Seidel (no email)]

Comment on attachment 49077 [details]
Patch to allow loading the QtLauncher with a -no-plugins option

Did you mean to set review=?  You are not a reviewer. :)  Reviewers set r+.

[Eric Seidel (no email)]

Comment on attachment 49077 [details]
Patch to allow loading the QtLauncher with a -no-plugins option

Every change requires a ChangeLog, please see:
http://webkit.org/coding/contributing.html

[Casper van Donderen]

Created attachment 49199 [details]
Patch including ChangeLog entry allowing starting QtLauncher without plugins

[Kenneth Rohde Christiansen]

Comment on attachment 49199 [details]
Patch including ChangeLog entry allowing starting QtLauncher without plugins


> +    if (gNoPlugins)
> +        QWebSettings::globalSettings()->setAttribute(QWebSettings::PluginsEnabled, false);
> +    else
> +        QWebSettings::globalSettings()->setAttribute(QWebSettings::PluginsEnabled, true);

QWebSettings::globalSettings()->setAttribute(QWebSettings::PluginsEnabled, !gNoPlugins); would be preferred

[Casper van Donderen]

Created attachment 49218 [details]
Patch with reviewers comments applied.

[Kenneth Rohde Christiansen]

Comment on attachment 49218 [details]
Patch with reviewers comments applied.


> +    handleUserOptions();
> +
>      applyDefaultSettings();
>  
> -    handleUserOptions();

This change was not mentioned in the ChangeLog and could overwrite user set options by the defaults, which I don't this is what we want. Please clarify.

[Casper van Donderen]

As readable in Comment 1 it is at this time not possible to overwrite user enabled settings with default ones, since non of the default settings are settable by the user.

[Kenneth Rohde Christiansen]

(In reply to comment #9)
> As readable in Comment 1 it is at this time not possible to overwrite user
> enabled settings with default ones, since non of the default settings are
> settable by the user.

That was in Comment 0 :-) Btw, that still does not explain why you changes those lines. Please do not make changes not related to your patch.

[Casper van Donderen]

Well,

if I first enable the default settings (when gNoPlugins = 0) you can never start the application without the plugins.

So either it became:
if (args.contains("-no-plugins"))
    QWebSettings::globalSettings()->setAttribute(QWebSettings::PluginsEnabled, false);

or change the order to

handleUserOptions();
applyDefaultSettings();

Since there will be no harm from the second option and the first option seemed slower (which would set the attribute twice) I decided to do it like this.

[Kenneth Rohde Christiansen]

You should be able to override the setting per page. Only if the setting is not set on the page it will use the global one.

[Robert Hogan]

Wouldn't this be better under Develop->Enable Plugins, or even better Edit->Settings->Enable Plugins?

Seems what's really missing is a way of changing page settings on the fly.

[Adam Barth]

Comment on attachment 49218 [details]
Patch with reviewers comments applied.

We now have the technology to enable/disable plugins on a per-frame basis.  Please use FrameLoaderClient::allowPlugins instead (as suggested above).

[Srikumar B]

Now i see the option to disable/enable plugins per page using Settings->Disable Plugins in QtTestBrowser. Does adding "no-plugins" to command line still make sense as a global option? 

If so, kindly reply by which i would like to work on making this change

-Sri

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.