[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 #9 from chris fleizach <cfleizach at apple.com> ---
Comment on attachment 367790
  --> https://bugs.webkit.org/attachment.cgi?id=367790
Patch

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

> Source/WebCore/ChangeLog:9
> +	- Split the existing SelectTextWithCriteria API into SearchTextWithCriteria and TextOperation.

8 spaces indent for these lines

> Source/WebCore/accessibility/AccessibilityObject.cpp:769
> +    ASSERT(!afterRange || afterRange->compareBoundaryPoints(Range::START_TO_START, *referenceRange).releaseReturnValue() >= 0);

whats the benefit of doing this assert over the previous one?

> Source/WebCore/accessibility/AccessibilityObject.cpp:846
> +RefPtr<Range> AccessibilityObject::findTextRange(Vector<String> const& searchStrings, RefPtr<Range> const& start, AccessibilitySearchTextDirection dir) const

dir -> direction

> Source/WebCore/accessibility/AccessibilityObject.cpp:854
> +        RefPtr<Range> foundAfter = rangeOfStringClosestToRangeInDirection(start.get(), AccessibilitySearchDirection::Next, searchStrings);

auto foundAfter
auto foundBefore

> Source/WebCore/accessibility/AccessibilityObject.cpp:882
> +        /* Collapse the range to the start unless searching from the end of the

I would make this a one line comment with //

> Source/WebCore/accessibility/AccessibilityObject.cpp:899
> +        found = findTextRange(criteria.searchStrings, startRange, criteria.direction);

we can probably write this is as

if (auto found = findTextRange(criteria.searchStrings, startRange, criteria.direction))
    result.append(found)

> Source/WebCore/accessibility/AccessibilityObject.cpp:905
> +            found = findTextRange(criteria.searchStrings, startRange, dir);

auto found

> Source/WebCore/accessibility/AccessibilityObject.cpp:922
> +Vector<String> AccessibilityObject::performTextOperation(AccessibilityTextOperation const& op)

op -> operation

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

auto* frame

> Source/WebCore/accessibility/AccessibilityObject.cpp:930
> +    if (!op.textRanges.isEmpty()) {

early return

if (!operation.textRanges.isEmpty()
   return result

> Source/WebCore/accessibility/AccessibilityObject.cpp:1129
>      Node* node = this->node();

auto* node

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:617
> +    AccessibilityTextOperation op;

op -> operation

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:658
> +static AccessibilitySearchTextCriteria accessibilitySearchTextCriteriaForParameterizedAttribute(const NSDictionary* params)

objective * character should be on the right side (which follows ObjC guidelines instead of C++ guidelines. confusing I know!)

NSDictionary *pararms

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:662
> +    NSArray* searchStrings = [params objectForKey:NSAccessibilitySearchTextSearchStrings];

*searchStrings
 *start
 *direction

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:669
> +        for (NSString* searchString in searchStrings) {

*searchString

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:694
> +static AccessibilityTextOperation accessibilityTextOperationForParameterizedAttribute(WebAccessibilityObjectWrapper* obj, const NSDictionary *parameterizedAttribute)

*obj
 *parameterizedAttrs

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:696
> +    AccessibilityTextOperation op;

op = operation

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:698
> +    NSArray* markerRanges = [parameterizedAttribute objectForKey:NSAccessibilityTextOperationMarkerRanges];

*markerRanges
 *operationType
 *replacementString

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4177
> +        return (NSString*)String();

I think this is by default, I don't think you need to do this cast

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4182
> +        Vector<RefPtr<Range>> ranges = m_object->findTextRanges(criteria);

auto ranges

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4186
> +                [markers addObject:[self textMarkerRangeFromRange:range]];

can textMarkerRangeFromRange return nil? if so we should validate before adding to object

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4194
> +        Vector<String> opResult = m_object->performTextOperation(textOperation);

operationResult

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4196
> +            NSMutableArray* result = [NSMutableArray arrayWithCapacity:opResult.size()];

*result

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4197
> +            for (String str : opResult)

for (auto str : operationResult)

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4198
> +                [result addObject:(NSString*)str];

cast should be unnecessary

> Tools/ChangeLog:8
> +	- WebCore-JavaScript glue for SearchText.

8 character indent

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:222
> +static Vector<T> convertNSArrayToVector(NSArray* a)

*array

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:288
>  

*searchTextParameterizedAttributeForCriteria

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:291
> +    NSMutableDictionary* parameterizedAttribute = [NSMutableDictionary dictionary];

*parameterizedAttribute

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:1262
> +    columnHeadersVector = convertNSArrayToVector<RefPtr<AccessibilityUIElement>>(columnHeadersArray);

auto columnHeaders

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:1273
> +    Vector<RefPtr<AccessibilityUIElement>> rowHeadersVector = convertNSArrayToVector<RefPtr<AccessibilityUIElement>>(rowHeadersArray);

auto rowHeadersVector

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:1284
> +    Vector<RefPtr<AccessibilityUIElement> > columnsVector = convertNSArrayToVector<RefPtr<AccessibilityUIElement>>(columnsArray);

auto columnVector

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:1295
> +    Vector<RefPtr<AccessibilityUIElement>> rowsVector = convertNSArrayToVector<RefPtr<AccessibilityUIElement>>(rowsArray);

auto

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:1306
> +    Vector<RefPtr<AccessibilityUIElement>> cellsVector = convertNSArrayToVector<RefPtr<AccessibilityUIElement>>(cellsArray);

auto

> LayoutTests/ChangeLog:8
> +	- Added LayoutTest for AccessibilitySearchText API.

8 character indent

-- 
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/20190419/554c3424/attachment-0001.html>