[Webkit-unassigned] [Bug 34539] Implement NPN_GetValueForURL and NPN_SetValueForURL and provide a stub for NPN_GetAuthenticationInfo

[bugzilla-daemon at webkit.org]

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


Simon Hausmann <hausmann at webkit.org> changed:

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




--- Comment #39 from Simon Hausmann <hausmann at webkit.org>  2010-08-30 00:22:01 PST ---
(From update of attachment 65051)
> WebCore/ChangeLog:3
> +        Reviewed by Andreas Kling 
Until the patch is landed this line should continue to say NOBODY (OOPS!).

> :722
> +bool PluginView::platformGetValueForURL(NPNURLVariable variable, const char* url, char** value, uint32_t* len, NPError* result)
Shouldn't this function use the QNetworkAccessManager's proxy settings instead of the application settings?

> WebCore/plugins/symbian/PluginViewSymbian.cpp:341
> +bool PluginView::platformGetValueForURL(NPNURLVariable variable, const char* url, char** value, uint32_t* len, NPError* result)
The current code duplication between the views sucks a bit, but in this "Symbian" implementation we should use the Qt bits and pieces. Qt is the system toolkit on Symbian.

I think overall this patch looks okay now, I agree with Andreas. However there is one thing missing: Test coverage for the new code. At least the cookie bits should be relatively easy to cover with the test netscape plugin.

I'm going to say r- primarily because of missing test coverage.

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