[Webkit-unassigned] [Bug 33180] [Qt] Support private browsing mode in plugins

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33180


Simon Hausmann <hausmann at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #45828|review?                     |review-
               Flag|                            |




--- Comment #4 from Simon Hausmann <hausmann at webkit.org>  2010-01-22 13:46:07 PST ---
(From update of attachment 45828)
> --- a/LayoutTests/platform/gtk/Skipped
> +++ b/LayoutTests/platform/gtk/Skipped
> @@ -3651,6 +3651,11 @@ plugins/plugin-javascript-access.html
>  # https://bugs.webkit.org/show_bug.cgi?id=30561
>  plugins/private-browsing-mode.html
>  
> +# https://bugs.webkit.org/show_bug.cgi?id=30561
> +plugins/private-browsing-mode.html

It seems strange to see these two lines being added that exist alread three
lines further up :)

> +# https://bugs.webkit.org/show_bug.cgi?id=33180
> +plugins/private-browsing-mode-2.html
> +
>  #   Tests generating new results
>  plugins/embed-attributes-style.html
>  plugins/netscape-dom-access.html


> +Tests that NPNVprivateModeBool is supported by the WebKit plugin view. This test is for WebKit platforms that wish to support NPNVprivateModeBool but do not wish to implement the preference change listener required to support a cachedPrivateBrowsingEnabled property similar to the one provided by Safari and tested for in private-browsing-mode.html

I don't understand this part. It seems trivial to implement in
DumpRenderTree/unix/TestNetscapePlugin/TestNetscapePlugin.cpp. Why not do it
there instead of this extra test?

In fact, it seems like a bug right now that the cachedPrivateBrowsingEnabled
variable isn't initialized in that file, where as it is initialized in
TestNetscapePlugIn.subproj/main.cpp. Could that be why the test was failing for
you?

> +
> +    case NPNVprivateModeBool: {
> +        Page* page = m_parentFrame->page();
> +        if (!page)
> +            return NPERR_GENERIC_ERROR;
> +         *((NPBool*)value) = (!page->settings() || page->settings()->privateBrowsingEnabled())? true : false;
> +         return NPERR_NO_ERROR;
> +    }

I would leave out the ? true : false; part and the extra parentheses, it's
redundant.

Otherwise the patch looks great, and certainly long-hanging fruit for the other
PluginView implementations, too.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.