[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


Adam Barth <abarth at webkit.org> changed:

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




--- Comment #29 from Adam Barth <abarth at webkit.org>  2010-06-21 19:20:48 PST ---
(From update of attachment 55729)
This patch needs a lot of love.

WebCore/ChangeLog:6
 +          NPN_GetAuthenticationInfo,
This ChangeLog doesn't tell me any of the "why" behind this patch.  I can see the "what" by looking at the code, but I don't understand what problem we're solving with this patch.

WebCore/plugins/npapi.cpp:196
 +          KURL URL = KURL(ParsedURLString, url); // NSURL *URL = [self URLWithCString:url];
Why would |url| be a ParsedURLString?  That doesn't seem right.

WebCore/plugins/npapi.cpp:205
 +                  strcpy(*value, cookieStringUTF8.data());
please use strncpy !

WebCore/plugins/npapi.cpp:220
 +  #if defined(BUILDING_QT__)
Surely this is the wrong preprocessor check.  Also, why do we have Qt-specific code in a non-Qt-specific file?  I think we need some better kind of abstraction here.

WebCore/plugins/npapi.cpp:252
 +          memcpy(*value, proxyUTF8.data(), proxyUTF8.length());
Why do you use memcpy here but strcpy above?  One of the two must be wrong.

WebCore/plugins/npapi.cpp:289
 +      }
You should have an explicit default case in to handle garbage from our caller.

WebCore/plugins/npapi.cpp:259
 +      }
You should have an explicit default case in to handle garbage from our caller.

WebCore/plugins/npapi.cpp:271
 +              KURL URL = KURL(ParsedURLString, url); // NSURL *URL = [self URLWithCString:url];
same ParsedURLString comment here.

WebCore/plugins/npapi.cpp:273
 +                  String cookieString = String::fromUTF8(value, len);
Are you sure this is supposed to be UTF8?

WebCore/plugins/npapi.cpp:276
 +                      Frame* frame = pluginViewForInstance(instance)->parentFrame();
parentFrame?  Could be, but looks scary.

WebCore/plugins/npapi.cpp:278
 +                          setCookies(frame->document(), URL, cookieString);
URL should be in lower case.

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