[Webkit-unassigned] [Bug 217869] Minor additions to minibrowser

[bugzilla-daemon at webkit.org]

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

--- Comment #6 from webkit at highwindsmedia.com ---
(In reply to Darin Adler from comment #5)
> Comment on attachment 411655 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=411655&action=review
> 
> > Tools/MiniBrowser/mac/SettingsController.h:62
> > + at property (nonatomic, readonly) BOOL useInvalidHTTPSWebsites;
> 
> I would call this "allow", not "use".
> 
> > Tools/MiniBrowser/mac/SettingsController.m:178
> > +    [self _addItemWithTitle:@"Enable accessing sites with an invalid HTTPS configuration" action:@selector(toggleUseInvalidHTTPSWebsites:) indented:YES];
> 
> This needs a shorter title. I think "Allow Invalid HTTPS" would be a good
> title.
> 
Done.
> > Tools/MiniBrowser/mac/WK2BrowserWindowController.m:795
> > +    SettingsController *settingsController = [[NSApplication sharedApplication] browserAppDelegate].settingsController;
> 
> No need to mix styles here. Could just write
> NSApplication.sharedApplication.browserAppDelegate.settingsController.
> 
> > Tools/MiniBrowser/mac/WK2BrowserWindowController.m:797
> > +    if ([challenge protectionSpace].serverTrust
> > +        && settingsController.useInvalidHTTPSWebsites) {
> 
> WebKit coding style would put this on one line, not broken like this.

done.

> 
> > Tools/MiniBrowser/mac/WK2BrowserWindowController.m:800
> > +        NSURLCredential *httpsCred =
> > +        [[NSURLCredential alloc] initWithTrust:[challenge protectionSpace].serverTrust];
> 
> This is not WebKit project style formatting, breaking the line like this. I
> suggest doing it in one line. I think credential is a fine name for this
> local variable, fitting more with WebKit coding style.
> 

Will do. I did skim through the code style guide, but it is clear that I would need to go through it in depth before my next patch.

> This object is being leaked here. This file does not get compiled with ARC,
> so we need to call release on httpsCred before returning.

My obj-c is rusty.  Will fix. Sorry about that.

Note: I didn't implement this for the Webkit1 viewer. Should I add this option to the webkit2-only settings menu, try to implement it for the Webkit1 viewer, or just leave as is?

thanks.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20201018/b3e8ad77/attachment.htm>