Bug 128397 - AX: Add text replacement activity support to NSAccessibilitySelectTextWithCriteriaParameterizedAttribute.
Summary: AX: Add text replacement activity support to NSAccessibilitySelectTextWithCri...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.9
: P2 Normal
Assignee: Samuel White
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-02-07 14:12 PST by Samuel White
Modified: 2014-02-11 12:42 PST (History)
11 users (show)

See Also:


Attachments
Patch. (26.65 KB, patch)
2014-02-10 14:23 PST, Samuel White
no flags Details | Formatted Diff | Diff
Updated patch. (26.65 KB, patch)
2014-02-10 14:32 PST, Samuel White
cfleizach: review+
Details | Formatted Diff | Diff
Updated patch to fix build issues. (27.59 KB, patch)
2014-02-10 15:26 PST, Samuel White
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (943.56 KB, application/zip)
2014-02-10 16:02 PST, Build Bot
no flags Details
More build fixing. (27.60 KB, patch)
2014-02-10 16:22 PST, Samuel White
no flags Details | Formatted Diff | Diff
Updated patch to fix more build issues. (28.00 KB, patch)
2014-02-10 17:02 PST, Samuel White
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (539.76 KB, application/zip)
2014-02-10 19:14 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (524.58 KB, application/zip)
2014-02-10 19:29 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (493.21 KB, application/zip)
2014-02-10 20:27 PST, Build Bot
no flags Details
Fixed patch. (28.00 KB, patch)
2014-02-10 22:03 PST, Samuel White
cfleizach: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel White 2014-02-07 14:12:40 PST
The above parameterized attribute should support the NSAccessibilitySelectTextActivityFindAndReplace activity.
Comment 1 Radar WebKit Bug Importer 2014-02-07 14:13:06 PST
<rdar://problem/16015807>
Comment 2 Samuel White 2014-02-10 14:23:48 PST
Created attachment 223745 [details]
Patch.
Comment 3 Samuel White 2014-02-10 14:32:50 PST
Created attachment 223750 [details]
Updated patch.

Fixing small bit of faulty logic.
Comment 4 Samuel White 2014-02-10 15:26:13 PST
Created attachment 223753 [details]
Updated patch to fix build issues.
Comment 5 Build Bot 2014-02-10 16:02:33 PST
Comment on attachment 223753 [details]
Updated patch to fix build issues.

Attachment 223753 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5947622191792128

New failing tests:
editing/unsupported-content/list-type-before.html
editing/unsupported-content/table-type-before.html
editing/unsupported-content/list-delete-001.html
platform/mac/accessibility/select-text.html
editing/unsupported-content/list-type-after.html
editing/unsupported-content/table-type-after.html
Comment 6 Build Bot 2014-02-10 16:02:35 PST
Created attachment 223760 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 7 Samuel White 2014-02-10 16:22:18 PST
Created attachment 223764 [details]
More build fixing.
Comment 8 Samuel White 2014-02-10 17:02:41 PST
Created attachment 223771 [details]
Updated patch to fix more build issues.
Comment 9 Build Bot 2014-02-10 19:14:51 PST
Comment on attachment 223771 [details]
Updated patch to fix more build issues.

Attachment 223771 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6744245880750080

New failing tests:
platform/mac/accessibility/select-text.html
Comment 10 Build Bot 2014-02-10 19:14:53 PST
Created attachment 223791 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 11 Build Bot 2014-02-10 19:29:36 PST
Comment on attachment 223771 [details]
Updated patch to fix more build issues.

Attachment 223771 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6259113621192704

New failing tests:
platform/mac/accessibility/select-text.html
Comment 12 Build Bot 2014-02-10 19:29:39 PST
Created attachment 223793 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 13 Build Bot 2014-02-10 20:27:04 PST
Comment on attachment 223771 [details]
Updated patch to fix more build issues.

Attachment 223771 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5015626099720192

New failing tests:
platform/mac/accessibility/select-text.html
Comment 14 Build Bot 2014-02-10 20:27:06 PST
Created attachment 223801 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 15 Samuel White 2014-02-10 22:03:22 PST
Created attachment 223807 [details]
Fixed patch.
Comment 16 Samuel White 2014-02-11 09:40:17 PST
Comment on attachment 223807 [details]
Fixed patch.

Fixed up the build issues. Should be good to go.
Comment 17 chris fleizach 2014-02-11 09:54:00 PST
Comment on attachment 223807 [details]
Fixed patch.

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

> Source/WebCore/accessibility/AccessibilityObject.h:406
> +    AccessibilitySelectTextCriteria(AccessibilitySelectTextActivity activity, AccessibilitySelectTextAmbiguityResolution ambiguityResolution, String replacementString)

I think this can be const String&, since we're going to copy it anyway with , replacementString(replacementString)

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:637
> +        if ([activityParameter isEqualToString:NSAccessibilitySelectTextActivityFindAndReplace])

you can probably put this if statement together with the one above
Comment 18 Samuel White 2014-02-11 12:13:15 PST
(In reply to comment #17)
> (From update of attachment 223807 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223807&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.h:406
> > +    AccessibilitySelectTextCriteria(AccessibilitySelectTextActivity activity, AccessibilitySelectTextAmbiguityResolution ambiguityResolution, String replacementString)
> 
> I think this can be const String&, since we're going to copy it anyway with , replacementString(replacementString)

Seems like a good optimization to me.

> 
> > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:637
> > +        if ([activityParameter isEqualToString:NSAccessibilitySelectTextActivityFindAndReplace])
> 
> you can probably put this if statement together with the one above

They are separate because each new activity will be a new if statement. See the other parameter handling around it.

I'll add the above optimization and commit directly. Thanks!
Comment 19 Samuel White 2014-02-11 12:42:28 PST
Patch landed:

http://trac.webkit.org/changeset/163899

Closing bug.