[Webkit-unassigned] [Bug 197095] Accessibility text search and selection API enhancements.

[bugzilla-daemon at webkit.org]

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

--- Comment #12 from chris fleizach <cfleizach at apple.com> ---
Comment on attachment 367899
  --> https://bugs.webkit.org/attachment.cgi?id=367899
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=367899&action=review

> Source/WebCore/ChangeLog:8
> +- Split the existing SelectTextWithCriteria API into two: search text API

bad indendation here

> Source/WebCore/accessibility/AccessibilityObject.cpp:914
> +        ASSERT(false);

you have all the cases covered? you probably you don't need the default case at all. compiler should take care of that for you

> Source/WebCore/accessibility/AccessibilityObject.cpp:927
> +    Frame* frame = this->frame();

auto* frame

> Source/WebCore/accessibility/AccessibilityObject.cpp:932
> +        if (frame->selection().setSelectedRange(textRange.get(), DOWNSTREAM, FrameSelection::ShouldCloseTyping::Yes)) {

should probably do early break here

if (!frame->selection().......)
     continue;

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:664
> +    NSString *dir = [params objectForKey:NSAccessibilitySearchTextDirection];

dir -> direction

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:699
> +    NSString *opType = [parameterizedAttribute objectForKey:NSAccessibilityTextOperationType];

opType -> operationType

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4183
> +        if (!ranges.isEmpty()) {

if (ranges.isEmpty()
   return nil;

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4197
> +        if (!operationResult.isEmpty()) {

if (operationResult.isEmpty())
    return nil;

> Tools/ChangeLog:8
> +- Added new API JS binding code for searchTextWithCriteria to both WTR and DRT.

bad indentation

> Tools/DumpRenderTree/AccessibilityUIElement.cpp:289
> +    JSStringRef startFrom = nullptr;

we should do a RefPtr<JSStringRef> so that we don't have to worry about the JSStringRelease at the end

> Tools/DumpRenderTree/AccessibilityUIElement.cpp:296
> +    auto result = toAXElement(thisObject)->searchTextWithCriteria(context, searchStrings, startFrom, direction);

then you can return immediately

return toAXElemen......

> Tools/DumpRenderTree/AccessibilityUIElement.cpp:1924
> +#if PLATFORM(MAC) && !PLATFORM(IOS_FAMILY)

I think MAC means it is not IOS_FAMILY, so no need to do the !PLATFORM(IOS_FAM

> Tools/DumpRenderTree/mac/AccessibilityUIElementMac.mm:317
> +    elementVector = convertNSArrayToVector<AccessibilityUIElement>(linkedElements);

is this going to work by setting elementVector? does it implicitly copy all elements into a vector by overloading = ?

> LayoutTests/ChangeLog:8
> +- Added new test for AccessibilitySearchTextWithCriteria API.

bad indentation

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