Bug 26638 - WebKitErrors.m: _initWithPluginErrorCode: does not set localizedDescription
Summary: WebKitErrors.m: _initWithPluginErrorCode: does not set localizedDescription
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Nobody
URL: http://content.eska.pl/streamplayers/...
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-22 20:23 PDT by opendarwin
Modified: 2009-06-24 01:00 PDT (History)
0 users

See Also:


Attachments
Patch (1.28 KB, patch)
2009-06-23 01:09 PDT, opendarwin
mrowe: review-
Details | Formatted Diff | Diff
Patch version 2 (1.61 KB, patch)
2009-06-23 02:01 PDT, opendarwin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description opendarwin 2009-06-22 20:23:32 PDT
Overview Description:
Most WebKit errors get a localizedDescription set in the method -[NSError _webkit_initWithDomain:code:URL]. However, plugin errors created with -[NSError _initWithPluginErrorCode:contentURL:pluginPageURL:pluginName:MIMEType:] do not get a localizedDescription set, so they end up having a generic "Operation could not be completed" localizedDescription, which is not good for the client app that has to present errors to the user.

Steps to Reproduce:
Load http://content.eska.pl/streamplayers/eska_rock/

Actual Results:
The <WebResourceLoadDelegate> method webView:plugInFailedWithError:dataSource: is called, but the localizedDescription of the error is the generic "Operation could not be completed. (WebKitErrorDomain error 200.)"

Expected Results:
The <WebResourceLoadDelegate> method webView:plugInFailedWithError:dataSource: is called, and the localizedDescription of the error is the string defined by WebKitErrorDescriptionCannotFindPlugin in WebKitErrors.m

Build Date & Platform:
git commit c12acbedcaab7593e406017bf40680761bef6ba8, corresponding to svn r44927 Mon Jun 22 01:39:12 2009 +0000
Comment 1 opendarwin 2009-06-23 01:09:05 PDT
Created attachment 31708 [details]
Patch
Comment 2 Mark Rowe (bdash) 2009-06-23 01:28:37 PDT
Comment on attachment 31708 [details]
Patch

> diff --git a/WebKit/mac/ChangeLog b/WebKit/mac/ChangeLog
> index c48775e..7091307 100644
> --- a/WebKit/mac/ChangeLog
> +++ b/WebKit/mac/ChangeLog
> @@ -1,3 +1,10 @@
> +2009-06-23  Jeff Johnson  <opendarwin@lapcatsoftware.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * Misc/WebKitErrors.m:
> +        (-[NSError _initWithPluginErrorCode:contentURL:pluginPageURL:pluginName:MIMEType:]):
> +
>  2009-06-20  Darin Adler  <darin@apple.com>

The ChangeLog should contain a link to the bug and it's title, along with a brief description of the change.  See other entries in the file for an example.

>          Reviewed by Sam Weinig.
> diff --git a/WebKit/mac/Misc/WebKitErrors.m b/WebKit/mac/Misc/WebKitErrors.m
> index 5985d9a..303c426 100644
> --- a/WebKit/mac/Misc/WebKitErrors.m
> +++ b/WebKit/mac/Misc/WebKitErrors.m
> @@ -106,6 +106,11 @@ static NSMutableDictionary *descriptions = nil;
>      [[self class] _registerWebKitErrors];
>      
>      NSMutableDictionary *userInfo = [[NSMutableDictionary alloc] init];
> +    NSDictionary *descriptionsDict = [descriptions objectForKey:WebKitErrorDomain];
> +    NSString *localizedDesc = descriptionsDict ? [descriptionsDict objectForKey:[NSNumber numberWithInt:code]] : nil;
> +    if (localizedDesc) {
> +        [userInfo setObject:localizedDesc forKey:NSLocalizedDescriptionKey];
> +    }
>      if (contentURL) {
>          [userInfo setObject:contentURL forKey:@"NSErrorFailingURLKey"];
>  #if defined(BUILDING_ON_TIGER) || defined(BUILDING_ON_LEOPARD)

I mentioned a few style issues with this code on IRC:

* Sending a message with an object return type to nil will return nil, so the ternary statement is not necessary
* Abbreviating Description to Desc is inconsistent with the other variable name, and not something we typically do
* We omit braces around a single-line if statement

For easy reference, the WebKit coding style guidelines are at <http://webkit.org/coding/coding-style.html>.

Besides the ChangeLog and style issues the change is good.  If you can make these changes and resubmit the patch I'll happily throw an r+ on it for you.
Comment 3 opendarwin 2009-06-23 02:01:22 PDT
Created attachment 31713 [details]
Patch version 2

Revised to match style guidelines.
Comment 4 Eric Seidel (no email) 2009-06-24 01:00:14 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/mac/ChangeLog
	M	WebKit/mac/Misc/WebKitErrors.m
Committed r45074
http://trac.webkit.org/changeset/45074