[Webkit-unassigned] [Bug 164437] Support TouchBar in WebKit

[bugzilla-daemon at webkit.org]

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

--- Comment #4 from Beth Dakin <bdakin at apple.com> ---
(In reply to comment #3)
> Comment on attachment 293959 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=293959&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:176
> > +#if HAVE(TOUCH_BAR)
> 
> I think this would look less ugly as its own class extension (you can have
> as many as you want!) but either way is fine.
> 

Good point. Fixed!

> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:4573
> > +    return NO;
> 
> If HAVE(TOUCH_BAR) is true, you return twice?
> 

Yeah, the compiler didn't complain, but I fixed it.

> > Source/WebKit2/UIProcess/API/mac/WKView.mm:1505
> > +    return NO;
> 
> And here.
> 

Fixed.

> > Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:971
> > +            NSTextAlignment textAlignment;
> > +            switch (editorState.postLayoutData().textAlignment) {
> > +            case NoAlignment:
> > +                textAlignment = NSTextAlignmentNatural;
> > +                break;
> 
> Should factor this out. Maybe we already have one?
> 

I don't believe it exists anywhere already. This will be a good follow-up.

> > Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:1111
> > +    CFNotificationCenterRemoveObserver(CFNotificationCenterGetLocalCenter(), this, (__bridge CFStringRef)NSTouchBarDidExitCustomization, 0);
> 
> __bridge? In non-ARC code?
> 

Good point. It's been bothering me that this uses CFNotificationCenter instead of NSNotificationCenter, so I have changed it to use NSNotificationCenter.

> > Source/WebKit/mac/WebView/WebView.mm:5136
> > +#if HAVE(TOUCH_BAR)
> 
> Why not around the whole thing?
> 

Fixed.

> > Source/WebKit/mac/WebView/WebView.mm:6865
> > +    NSRange resultRange = [nsResult range];
> 
> Dots.
> 

Fixed.

> > Source/WebKit/mac/WebView/WebView.mm:6868
> > +    result.replacement = [nsResult replacementString];
> 
> Dots.
> 

Fixed.

> > Source/WebKit/mac/WebView/WebView.mm:9465
> > +                NSTextAlignment textAlignment;
> > +                switch (style->textAlign()) {
> > +                case RIGHT:
> > +                case WEBKIT_RIGHT:
> > +                    textAlignment = NSTextAlignmentRight;
> 
> And again.

Will do in follow-up.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161105/734540aa/attachment-0001.html>