RESOLVED FIXED196636
AX: Support API: accessibilityReplaceRange:withText 
https://bugs.webkit.org/show_bug.cgi?id=196636
Summary AX: Support API: accessibilityReplaceRange:withText
chris fleizach
Reported 2019-04-04 17:07:02 PDT
Support accessibility platform API of accessibilityReplaceRange:withText <rdar://problem/49438073>
Attachments
patch (16.80 KB, patch)
2019-04-04 17:09 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
patch (17.86 KB, patch)
2019-04-04 17:10 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
patch (17.86 KB, patch)
2019-04-04 17:14 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
patch (17.86 KB, patch)
2019-04-05 13:00 PDT, chris fleizach 		dbates: review+
DetailsFormatted DiffDiff
patch (17.85 KB, patch)
2019-04-08 16:18 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
patch for landing (17.85 KB, patch)
2019-04-08 16:19 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
patch for landing (17.82 KB, patch)
2019-04-08 16:22 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
chris fleizach
Comment 1 2019-04-04 17:09:23 PDT
Created attachment 366776 [details] patch
chris fleizach
Comment 2 2019-04-04 17:10:47 PDT
Created attachment 366777 [details] patch
EWS Watchlist
Comment 3 2019-04-04 17:12:59 PDT
Attachment 366777 [details] did not pass style-queue: ERROR: Source/WebCore/accessibility/AccessibilityObject.h:297: The parameter name "r" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
chris fleizach
Comment 4 2019-04-04 17:14:31 PDT
Created attachment 366778 [details] patch
chris fleizach
Comment 5 2019-04-05 13:00:29 PDT
Created attachment 366835 [details] patch
Daniel Bates
Comment 6 2019-04-08 12:55:28 PDT
Comment on attachment 366835 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=366835&action=review > Source/WebCore/accessibility/AccessibilityObject.cpp:2287 > + if (auto frame = renderer()->document().frame()) { auto* please I could have sworn RnederObject has a frame() convenience method instead of deref document. > Source/WebCore/accessibility/AccessibilityObject.cpp:2296 > + downcast<HTMLInputElement>(element).setRangeText(replacementString, range.start, range.length, ""); Bad cast if element is HTMLTextAreaElement. > Tools/DumpRenderTree/AccessibilityUIElement.cpp:811 > + JSStringRef text = 0; nullptr? Don't we have some smart pointer like class we can use to manage this value. Pretty sure we have one. Can you take a look at other code that make use of JSStringRef? > Tools/DumpRenderTree/AccessibilityUIElement.cpp:820 > + JSValueRef result = JSValueMakeBoolean(context, toAXElement(thisObject)->replaceTextInRange(text, position, length)); Assuming this handles 0 for position/length. > Tools/DumpRenderTree/AccessibilityUIElement.cpp:822 > + if (text) > + JSStringRelease(text); Ok as-is. Smart pointer would be preferred. If we don't have one then does JSStringRelease() null check for us? > LayoutTests/accessibility/mac/replace-text-with-range-expected.txt:2 > +HelloBlorg > + Ok as-is. In my opinion this makes the test look a little messy. If I was writing this test I would figure out a way to omit this. > LayoutTests/accessibility/mac/replace-text-with-range.html:1 > +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN"> Old Doctype. Intentional? If not, use new hotness: <!DOCTYPE html> 😀 > LayoutTests/accessibility/mac/replace-text-with-range.html:4 > +<script src="../../resources/js-test-pre.js"></script> New hotness is to just include js-test.js and omit -pre and -post.js files.
chris fleizach
Comment 7 2019-04-08 16:18:39 PDT
Created attachment 366993 [details] patch all comments addressed. thanks
chris fleizach
Comment 8 2019-04-08 16:19:48 PDT
Created attachment 366994 [details] patch for landing
chris fleizach
Comment 9 2019-04-08 16:22:28 PDT
Created attachment 366995 [details] patch for landing
WebKit Commit Bot
Comment 10 2019-04-08 17:39:56 PDT
Comment on attachment 366995 [details] patch for landing Clearing flags on attachment: 366995 Committed r244059: <https://trac.webkit.org/changeset/244059>
WebKit Commit Bot
Comment 11 2019-04-08 17:39:58 PDT
All reviewed patches have been landed. Closing bug.
Fujii Hironori
Comment 12 2019-04-08 18:26:58 PDT
Committed r244062: <https://trac.webkit.org/changeset/244062>
Fujii Hironori
Comment 13 2019-04-08 18:54:59 PDT
Committed r244066: <https://trac.webkit.org/changeset/244066>
Note You need to log in before you can comment on or make changes to this bug.