Bug 166367 - Holding down on candidates in the TouchBar should show panel on screen
Summary: Holding down on candidates in the TouchBar should show panel on screen
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-21 12:50 PST by Beth Dakin
Modified: 2017-01-03 14:21 PST (History)
6 users (show)

See Also:

Attachments
Patch (8.33 KB, patch)
2016-12-21 12:59 PST, Beth Dakin 		thorton: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.