[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





--- Comment #40 from Dawit A. <adawit at kde.org>  2010-08-30 01:14:05 PST ---
(In reply to comment #39)
> (From update of attachment 65051 [details])
> > 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.

(In reply to comment #39)
> (From update of attachment 65051 [details])
> > 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?

Probably, but I am not entirely sure how I can access the QNetworkAccessManager's proxy from this implementation...

> > 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.

Well no generic way to access proxy information means that this is the only viable approach at the moment.

> 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.

Where does the test coverage go ? LayoutTests/plugin ?

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