26638 2009-06-22 20:23:32 -0700 WebKitErrors.m: _initWithPluginErrorCode: does not set localizedDescription 2009-06-24 01:00:14 -0700 1 1 1 Unclassified WebKit WebKit API 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED http://content.eska.pl/streamplayers/eska_rock/ P2 Normal --- 0 opendarwin webkit-unassigned oldest_to_newest 127555 0 opendarwin 2009-06-22 20:23:32 -0700 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 127588 1 31708 opendarwin 2009-06-23 01:09:05 -0700 Created attachment 31708 Patch 127590 2 31708 mrowe 2009-06-23 01:28:37 -0700 Comment on attachment 31708 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. 127607 3 31713 opendarwin 2009-06-23 02:01:22 -0700 Created attachment 31713 Patch version 2 Revised to match style guidelines. 127857 4 eric 2009-06-24 01:00:14 -0700 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 31708 2009-06-23 01:09:05 -0700 2009-06-23 02:01:22 -0700 Patch plugin-error-description.patch text/plain 1312 opendarwin ZGlmZiAtLWdpdCBhL1dlYktpdC9tYWMvQ2hhbmdlTG9nIGIvV2ViS2l0L21hYy9DaGFuZ2VMb2cK aW5kZXggYzQ4Nzc1ZS4uNzA5MTMwNyAxMDA2NDQKLS0tIGEvV2ViS2l0L21hYy9DaGFuZ2VMb2cK KysrIGIvV2ViS2l0L21hYy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMCBAQAorMjAwOS0wNi0yMyAg SmVmZiBKb2huc29uICA8b3BlbmRhcndpbkBsYXBjYXRzb2Z0d2FyZS5jb20+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBNaXNjL1dlYktpdEVycm9y cy5tOgorICAgICAgICAoLVtOU0Vycm9yIF9pbml0V2l0aFBsdWdpbkVycm9yQ29kZTpjb250ZW50 VVJMOnBsdWdpblBhZ2VVUkw6cGx1Z2luTmFtZTpNSU1FVHlwZTpdKToKKwogMjAwOS0wNi0yMCAg RGFyaW4gQWRsZXIgIDxkYXJpbkBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgU2Ft IFdlaW5pZy4KZGlmZiAtLWdpdCBhL1dlYktpdC9tYWMvTWlzYy9XZWJLaXRFcnJvcnMubSBiL1dl YktpdC9tYWMvTWlzYy9XZWJLaXRFcnJvcnMubQppbmRleCA1OTg1ZDlhLi4zMDNjNDI2IDEwMDY0 NAotLS0gYS9XZWJLaXQvbWFjL01pc2MvV2ViS2l0RXJyb3JzLm0KKysrIGIvV2ViS2l0L21hYy9N aXNjL1dlYktpdEVycm9ycy5tCkBAIC0xMDYsNiArMTA2LDExIEBAIHN0YXRpYyBOU011dGFibGVE aWN0aW9uYXJ5ICpkZXNjcmlwdGlvbnMgPSBuaWw7CiAgICAgW1tzZWxmIGNsYXNzXSBfcmVnaXN0 ZXJXZWJLaXRFcnJvcnNdOwogICAgIAogICAgIE5TTXV0YWJsZURpY3Rpb25hcnkgKnVzZXJJbmZv ID0gW1tOU011dGFibGVEaWN0aW9uYXJ5IGFsbG9jXSBpbml0XTsKKyAgICBOU0RpY3Rpb25hcnkg KmRlc2NyaXB0aW9uc0RpY3QgPSBbZGVzY3JpcHRpb25zIG9iamVjdEZvcktleTpXZWJLaXRFcnJv ckRvbWFpbl07CisgICAgTlNTdHJpbmcgKmxvY2FsaXplZERlc2MgPSBkZXNjcmlwdGlvbnNEaWN0 ID8gW2Rlc2NyaXB0aW9uc0RpY3Qgb2JqZWN0Rm9yS2V5OltOU051bWJlciBudW1iZXJXaXRoSW50 OmNvZGVdXSA6IG5pbDsKKyAgICBpZiAobG9jYWxpemVkRGVzYykgeworICAgICAgICBbdXNlcklu Zm8gc2V0T2JqZWN0OmxvY2FsaXplZERlc2MgZm9yS2V5Ok5TTG9jYWxpemVkRGVzY3JpcHRpb25L ZXldOworICAgIH0KICAgICBpZiAoY29udGVudFVSTCkgewogICAgICAgICBbdXNlckluZm8gc2V0 T2JqZWN0OmNvbnRlbnRVUkwgZm9yS2V5OkAiTlNFcnJvckZhaWxpbmdVUkxLZXkiXTsKICNpZiBk ZWZpbmVkKEJVSUxESU5HX09OX1RJR0VSKSB8fCBkZWZpbmVkKEJVSUxESU5HX09OX0xFT1BBUkQp Cg== 31713 2009-06-23 02:01:22 -0700 2009-06-23 10:04:42 -0700 Patch version 2 plugin-error-description2.patch text/plain 1649 opendarwin ZGlmZiAtLWdpdCBhL1dlYktpdC9tYWMvQ2hhbmdlTG9nIGIvV2ViS2l0L21hYy9DaGFuZ2VMb2cK aW5kZXggYzQ4Nzc1ZS4uZjU5ODY1MCAxMDA2NDQKLS0tIGEvV2ViS2l0L21hYy9DaGFuZ2VMb2cK KysrIGIvV2ViS2l0L21hYy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAwOS0wNi0yMyAg SmVmZiBKb2huc29uICA8b3BlbmRhcndpbkBsYXBjYXRzb2Z0d2FyZS5jb20+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTI2NjM4IFdlYktpdEVycm9ycy5tOiBfaW5pdFdpdGhQbHVn aW5FcnJvckNvZGU6IGRvZXMgbm90IHNldCBsb2NhbGl6ZWREZXNjcmlwdGlvbgorCisgICAgICAg IEFkZCBsb2NhbGl6ZWQgZGVzY3JpcHRpb25zIGZvciBwbHVnaW4gZXJyb3JzLgorICAgICAgICBU aGUgbG9jYWxpemVkIHN0cmluZ3MgYWxyZWFkeSBleGlzdGVkIGJ1dCB3ZXJlIHVudXNlZDsgbm93 IE5TTG9jYWxpemVkRGVzY3JpcHRpb25LZXkgaXMgYWRkZWQgdG8gdGhlIE5TRXJyb3IgdXNlcklu Zm8uCisKKyAgICAgICAgKiBNaXNjL1dlYktpdEVycm9ycy5tOgorICAgICAgICAoLVtOU0Vycm9y IF9pbml0V2l0aFBsdWdpbkVycm9yQ29kZTpjb250ZW50VVJMOnBsdWdpblBhZ2VVUkw6cGx1Z2lu TmFtZTpNSU1FVHlwZTpdKToKKwogMjAwOS0wNi0yMCAgRGFyaW4gQWRsZXIgIDxkYXJpbkBhcHBs ZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgU2FtIFdlaW5pZy4KZGlmZiAtLWdpdCBhL1dl YktpdC9tYWMvTWlzYy9XZWJLaXRFcnJvcnMubSBiL1dlYktpdC9tYWMvTWlzYy9XZWJLaXRFcnJv cnMubQppbmRleCA1OTg1ZDlhLi5lYzQyZGM5IDEwMDY0NAotLS0gYS9XZWJLaXQvbWFjL01pc2Mv V2ViS2l0RXJyb3JzLm0KKysrIGIvV2ViS2l0L21hYy9NaXNjL1dlYktpdEVycm9ycy5tCkBAIC0x MDYsNiArMTA2LDEwIEBAIHN0YXRpYyBOU011dGFibGVEaWN0aW9uYXJ5ICpkZXNjcmlwdGlvbnMg PSBuaWw7CiAgICAgW1tzZWxmIGNsYXNzXSBfcmVnaXN0ZXJXZWJLaXRFcnJvcnNdOwogICAgIAog ICAgIE5TTXV0YWJsZURpY3Rpb25hcnkgKnVzZXJJbmZvID0gW1tOU011dGFibGVEaWN0aW9uYXJ5 IGFsbG9jXSBpbml0XTsKKyAgICBOU0RpY3Rpb25hcnkgKmRlc2NyaXB0aW9uc0ZvcldlYktpdEVy cm9yRG9tYWluID0gW2Rlc2NyaXB0aW9ucyBvYmplY3RGb3JLZXk6V2ViS2l0RXJyb3JEb21haW5d OworICAgIE5TU3RyaW5nICpsb2NhbGl6ZWREZXNjcmlwdGlvbiA9IFtkZXNjcmlwdGlvbnNGb3JX ZWJLaXRFcnJvckRvbWFpbiBvYmplY3RGb3JLZXk6W05TTnVtYmVyIG51bWJlcldpdGhJbnQ6Y29k ZV1dOworICAgIGlmIChsb2NhbGl6ZWREZXNjcmlwdGlvbikKKyAgICAgICAgW3VzZXJJbmZvIHNl dE9iamVjdDpsb2NhbGl6ZWREZXNjcmlwdGlvbiBmb3JLZXk6TlNMb2NhbGl6ZWREZXNjcmlwdGlv bktleV07CiAgICAgaWYgKGNvbnRlbnRVUkwpIHsKICAgICAgICAgW3VzZXJJbmZvIHNldE9iamVj dDpjb250ZW50VVJMIGZvcktleTpAIk5TRXJyb3JGYWlsaW5nVVJMS2V5Il07CiAjaWYgZGVmaW5l ZChCVUlMRElOR19PTl9USUdFUikgfHwgZGVmaW5lZChCVUlMRElOR19PTl9MRU9QQVJEKQo=