Bug 166367

Summary: Holding down on candidates in the TouchBar should show panel on screen
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: WebKit Misc.Assignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, dbates, sam, thorton, wenson_hsieh
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch thorton: review+

Description Beth Dakin 2016-12-21 12:50:40 PST
Holding down on candidates in the TouchBar should show panel on screen

rdar://problem/28479236
Comment 1 Beth Dakin 2016-12-21 12:59:31 PST
Created attachment 297619 [details]
Patch
Comment 2 Beth Dakin 2016-12-21 13:31:37 PST
Thanks Tim! https://trac.webkit.org/changeset/210075
Comment 3 Daniel Bates 2017-01-03 12:33:37 PST
(In reply to comment #2)
> Thanks Tim! https://trac.webkit.org/changeset/210075

This broke the Apple Sierra Release 32-bit build, which builds without Touch Bar support:

[[
release/build/Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:2991:6: error: use of undeclared identifier 'candidateListTouchBarItem'
    [candidateListTouchBarItem() setCandidates:candidates forSelectedRange:selectedRange inString:postLayoutData.paragraphContextForCandidateRequest rect:offsetSelectionRect view:m_view completionHandler:nil];
     ^
]]
<https://build.webkit.org/builders/Apple%20Sierra%20Release%20%2832-bit%20Build%29/builds/2528/steps/compile-webkit/logs/stdio>
Comment 4 Daniel Bates 2017-01-03 12:35:30 PST
(In reply to comment #3)
> (In reply to comment #2)
> > Thanks Tim! https://trac.webkit.org/changeset/210075
> 
> This broke the Apple Sierra Release 32-bit build, which builds without Touch
> Bar support:
> 
> [[
> release/build/Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm:2991:6: error:
> use of undeclared identifier 'candidateListTouchBarItem'
>     [candidateListTouchBarItem() setCandidates:candidates
> forSelectedRange:selectedRange
> inString:postLayoutData.paragraphContextForCandidateRequest
> rect:offsetSelectionRect view:m_view completionHandler:nil];
>      ^
> ]]
> <https://build.webkit.org/builders/Apple%20Sierra%20Release%20%2832-
> bit%20Build%29/builds/2528/steps/compile-webkit/logs/stdio>

Committed build fix in <https://trac.webkit.org/changeset/210245>.
Comment 5 Daniel Bates 2017-01-03 12:44:14 PST
We should look to extract out the Touch Bar code from Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm or at least consolidate it at one end of this file. Currently the Touch Bar code is scattered throughout this file and in #if/else chunks that are many lines long. As demonstrated by the Apple Sierra Release 32-bit build failure (comment #3) it is easy to lose track of what chunk in the file is HAVE(TOUCH_BAR)-guarded or not. I needed to use an editor that supports collapsing code regions in order to be able to track the #if HAVE(TOUCH_BAR) and #else chunks. This significantly affects the hackability of this file and makes refactoring code in it error prone.
Comment 6 Daniel Bates 2017-01-03 12:55:03 PST
Committed another attempt to fix the Apple Sierra Release 32-bit build in <https://trac.webkit.org/changeset/210247>.
Comment 7 Alexey Proskuryakov 2017-01-03 13:01:51 PST
We need to update Xcode on this bot; I don't think that this is a supported configuration any more.
Comment 8 Alexey Proskuryakov 2017-01-03 14:21:39 PST
Ryan took care of updating it.