208720 – [iOS] Implement support for dictation alternatives

RESOLVED FIXED 208720

[iOS] Implement support for dictation alternatives

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

Summary [iOS] Implement support for dictation alternatives

Daniel Bates

Reported 2020-03-06 09:42:10 PST

Implement support for dictation alternatives aka the purple dots.

Attachments
Part 1: For the bots (33.54 KB, patch) 2020-03-06 09:43 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 1: For the bots (33.56 KB, patch) 2020-03-06 09:54 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 2: For the bots (14.39 KB, patch) 2020-03-06 09:55 PST, Daniel Bates	no flags	Details Formatted Diff Diff
All in one - for the bots (67.41 KB, patch) 2020-03-07 00:23 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 1: Share AlternativeTextUIController (40.16 KB, patch) 2020-03-07 10:40 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 1: Share AlternativeTextUIController (40.29 KB, patch) 2020-03-07 10:56 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 1: Share AlternativeTextUIController (41.54 KB, patch) 2020-03-07 10:59 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files (15.11 KB, patch) 2020-03-07 11:06 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 1: Share AlternativeTextUIController (42.19 KB, patch) 2020-03-07 11:14 PST, Daniel Bates	wenson_hsieh: review+	Details Formatted Diff Diff
Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files (15.31 KB, patch) 2020-03-07 11:17 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files (19.28 KB, patch) 2020-03-07 11:37 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files (19.08 KB, patch) 2020-03-07 12:35 PST, Daniel Bates	wenson_hsieh: review+	Details Formatted Diff Diff
Part 3: Share more AlternativeTextController code and WebPageProxy code (22.45 KB, patch) 2020-03-07 12:47 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 3: Share more AlternativeTextController code and WebPageProxy code (22.58 KB, patch) 2020-03-07 12:52 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 3: Share more AlternativeTextController code and WebPageProxy code (27.83 KB, patch) 2020-03-07 13:25 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 3: Share more AlternativeTextController code and WebPageProxy code (26.78 KB, patch) 2020-03-07 13:40 PST, Daniel Bates	bfulgham: review+	Details Formatted Diff Diff
Part 4: Implement -insertText:alternatives:style: (3.09 KB, patch) 2020-03-07 13:41 PST, Daniel Bates	beidson: review+	Details Formatted Diff Diff
Part 5: Enable on iOS <- easy review patch (1.42 KB, patch) 2020-03-07 13:46 PST, Daniel Bates	wenson_hsieh: review+	Details Formatted Diff Diff
[Just for bots] All in one patch (77.48 KB, patch) 2020-03-07 13:48 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 1: Share AlternativeTextUIController – To Land (40.69 KB, patch) 2020-03-07 16:17 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files – To Land (18.89 KB, patch) 2020-03-07 16:18 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 3: Share more AlternativeTextController code and WebPageProxy code – To Land (26.24 KB, patch) 2020-03-07 16:18 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 4: Implement -insertText:alternatives:style: – To Land (4.59 KB, patch) 2020-03-07 16:19 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Part 5: Enable on iOS – To Land (1.31 KB, patch) 2020-03-07 16:19 PST, Daniel Bates	no flags	Details Formatted Diff Diff
Show Obsolete (19) View All Add attachment proposed patch, testcase, etc.

Daniel Bates

Comment 1 2020-03-06 09:43:44 PST

Created attachment 392732 [details] Part 1: For the bots

Daniel Bates

Comment 2 2020-03-06 09:54:08 PST

Created attachment 392735 [details] Part 1: For the bots

Daniel Bates

Comment 3 2020-03-06 09:55:23 PST

Created attachment 392737 [details] Part 2: For the bots

Daniel Bates

Comment 4 2020-03-07 00:23:53 PST

Created attachment 392854 [details] All in one - for the bots

Daniel Bates

Comment 5 2020-03-07 10:40:18 PST

Created attachment 392866 [details] Part 1: Share AlternativeTextUIController

Daniel Bates

Comment 6 2020-03-07 10:56:08 PST

Created attachment 392868 [details] Part 1: Share AlternativeTextUIController

Daniel Bates

Comment 7 2020-03-07 10:59:52 PST

Created attachment 392869 [details] Part 1: Share AlternativeTextUIController Expose some more UIKit SPI

Daniel Bates

Comment 8 2020-03-07 11:06:03 PST

Created attachment 392870 [details] Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files

Daniel Bates

Comment 9 2020-03-07 11:14:39 PST

Created attachment 392872 [details] Part 1: Share AlternativeTextUIController

Daniel Bates

Comment 10 2020-03-07 11:17:01 PST

Created attachment 392873 [details] Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files

Daniel Bates

Comment 11 2020-03-07 11:37:41 PST

Created attachment 392874 [details] Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files

Daniel Bates

Comment 12 2020-03-07 12:35:27 PST

Created attachment 392876 [details] Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files

Daniel Bates

Comment 13 2020-03-07 12:47:45 PST

Created attachment 392878 [details] Part 3: Share more AlternativeTextController code and WebPageProxy code

Daniel Bates

Comment 14 2020-03-07 12:52:39 PST

Created attachment 392879 [details] Part 3: Share more AlternativeTextController code and WebPageProxy code

Wenson Hsieh

Comment 15 2020-03-07 12:53:16 PST

Comment on attachment 392872 [details] Part 1: Share AlternativeTextUIController View in context: https://bugs.webkit.org/attachment.cgi?id=392872&action=review > Source/WebCore/ChangeLog:14 > + Note that I haven't enable USE_DICTATION_ALTERNATIVES on iOS, yet. So, this code isn't being compile on it. Nit - "being compiled" > Source/WebKit/UIProcess/ios/PageClientImplIOS.h:37 > +OBJC_CLASS NSTextAlternatives; Nit - alphabetic order.

Daniel Bates

Comment 16 2020-03-07 13:25:40 PST

Created attachment 392883 [details] Part 3: Share more AlternativeTextController code and WebPageProxy code

Daniel Bates

Comment 17 2020-03-07 13:40:20 PST

Created attachment 392886 [details] Part 3: Share more AlternativeTextController code and WebPageProxy code

Daniel Bates

Comment 18 2020-03-07 13:41:24 PST

Created attachment 392887 [details] Part 4: Implement -insertText:alternatives:style:

Daniel Bates

Comment 19 2020-03-07 13:46:43 PST

Created attachment 392890 [details] Part 5: Enable on iOS <- easy review patch This patch will fail to build on EWS since it depends on the other 4 parts having been applied.

Daniel Bates

Comment 20 2020-03-07 13:48:19 PST

Created attachment 392891 [details] [Just for bots] All in one patch

Daniel Bates

Comment 21 2020-03-07 14:08:37 PST

<rdar://problem/58540114>

Wenson Hsieh

Comment 22 2020-03-07 15:42:10 PST

Comment on attachment 392876 [details] Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files View in context: https://bugs.webkit.org/attachment.cgi?id=392876&action=review > Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:259 > + options.registerUndoGroup = registerUndoGroup; (Not related to this patch) At some point, we should consider changing InsertTextOptions’s bool members into enum classes.

Brent Fulgham

Comment 23 2020-03-07 15:49:07 PST

Comment on attachment 392886 [details] Part 3: Share more AlternativeTextController code and WebPageProxy code View in context: https://bugs.webkit.org/attachment.cgi?id=392886&action=review r=me > Source/WebCore/ChangeLog:13 > + Note that I haven't enable USE_DICTATION_ALTERNATIVES on iOS. So, this code isn't being enabled

Daniel Bates

Comment 24 2020-03-07 16:17:44 PST

Created attachment 392904 [details] Part 1: Share AlternativeTextUIController – To Land

Daniel Bates

Comment 25 2020-03-07 16:18:27 PST

Created attachment 392905 [details] Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files – To Land

Daniel Bates

Comment 26 2020-03-07 16:18:47 PST

Created attachment 392906 [details] Part 3: Share more AlternativeTextController code and WebPageProxy code – To Land

Daniel Bates

Comment 27 2020-03-07 16:19:03 PST

Created attachment 392907 [details] Part 4: Implement -insertText:alternatives:style: – To Land

Daniel Bates

Comment 28 2020-03-07 16:19:35 PST

Created attachment 392908 [details] Part 5: Enable on iOS – To Land

Daniel Bates

Comment 29 2020-03-07 16:26:47 PST

Committed part 1 in r258085: <https://trac.webkit.org/changeset/258085>

Daniel Bates

Comment 30 2020-03-07 16:28:12 PST

Committed part 2 in r258086: <https://trac.webkit.org/changeset/258086>

Daniel Bates

Comment 31 2020-03-07 16:30:54 PST

Committed part 3 in r258087: <https://trac.webkit.org/changeset/258087>

Daniel Bates

Comment 32 2020-03-07 16:34:34 PST

Committed part 4 in r258088: <https://trac.webkit.org/changeset/258088>

Daniel Bates

Comment 33 2020-03-07 16:38:46 PST

Committed part 5 in r258089: <https://trac.webkit.org/changeset/258089>

Daniel Bates

Comment 34 2020-03-07 16:39:15 PST

(In reply to Brent Fulgham from comment #23) > > > Source/WebCore/ChangeLog:13 > > + Note that I haven't enable USE_DICTATION_ALTERNATIVES on iOS. So, this code isn't being > > enabled Oops! I forgot to do this

Simon Fraser (smfr)

Comment 35 2020-03-07 16:42:41 PST

Comment on attachment 392904 [details] Part 1: Share AlternativeTextUIController – To Land View in context: https://bugs.webkit.org/attachment.cgi?id=392904&action=review > Source/WebCore/ChangeLog:15 > + Note that I haven't enable USE_DICTATION_ALTERNATIVES on iOS. So, this code isn't being enabled > Source/WebCore/editing/cocoa/AlternativeTextContextController.h:47 > + NSTextAlternatives *alternativesForContext(uint64_t context); Can we 'uses' this uint64_t so we can tell it from all the other uint64_ts? or use ObjectIdentifier<> > Source/WebCore/editing/cocoa/AlternativeTextContextController.mm:47 > + uint64_t context = reinterpret_cast<uint64_t>(alternatives.get()); Ugh, the pointer address Is the id? Pointer addresses can get recycled. > Source/WebCore/editing/cocoa/AlternativeTextUIController.hSource/WebCore/editing/mac/AlternativeTextUIController.h:45 > + WEBCORE_EXPORT uint64_t addAlternatives(const RetainPtr<NSTextAlternatives>&); // Returns a context ID. If you used a named type you wouldn't need the comment.

Simon Fraser (smfr)

Comment 36 2020-03-07 16:43:58 PST

Comment on attachment 392905 [details] Part 2: Move insertDictatedTextAsync() from the Mac-specific files to Cocoa-specific files – To Land View in context: https://bugs.webkit.org/attachment.cgi?id=392905&action=review > Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:195 > + auto& frame = m_page->focusController().focusedOrMainFrame(); > + Ref<Frame> protector { frame }; Why not just Ref<Frame> = m_page->focusController().focusedOrMainFrame(); or fix focusedOrMainFrame() to return a ref

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Local Build

Hardware iPhone / iPad

OS iOS 13

Product WebKit

Component WebCore Misc.

Assignee

Daniel Bates

Reported

2020-03-06 09:42 PST

Modified

2020-03-07 16:43 PST History

CC List

10 users Show

URL

Keywords InRadar

Depends on

Blocks