Bug 160744 - AX: Spell check and style attributes should be optional when fetching attributed string
Summary: AX: Spell check and style attributes should be optional when fetching attribu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-08-10 13:09 PDT by Doug Russell
Modified: 2018-02-27 14:57 PST (History)
13 users (show)

See Also:


Attachments
Patch (37.21 KB, patch)
2016-08-10 13:29 PDT, Doug Russell
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (995.98 KB, application/zip)
2016-08-10 14:18 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (933.15 KB, application/zip)
2016-08-10 14:21 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-yosemite (1.49 MB, application/zip)
2016-08-10 14:30 PDT, Build Bot
no flags Details
Patch (51.37 KB, patch)
2016-08-10 15:25 PDT, Doug Russell
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-yosemite (1.04 MB, application/zip)
2016-08-10 16:05 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (921.31 KB, application/zip)
2016-08-10 16:16 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-yosemite (1.61 MB, application/zip)
2016-08-10 16:18 PDT, Build Bot
no flags Details
Patch (50.23 KB, patch)
2016-08-10 21:22 PDT, Doug Russell
no flags Details | Formatted Diff | Diff
Updated patch (32.73 KB, patch)
2018-02-26 19:49 PST, Doug Russell
no flags Details | Formatted Diff | Diff
Updated patch (36.82 KB, patch)
2018-02-26 20:03 PST, Doug Russell
no flags Details | Formatted Diff | Diff
Updated patch (36.12 KB, patch)
2018-02-26 20:09 PST, Doug Russell
no flags Details | Formatted Diff | Diff
Updated patch (40.07 KB, patch)
2018-02-26 20:14 PST, Doug Russell
no flags Details | Formatted Diff | Diff
Updated patch (36.50 KB, patch)
2018-02-27 13:02 PST, Doug Russell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Doug Russell 2016-08-10 13:09:46 PDT
The overhead of fetching a spell checked attributed string via AX API is substantial. In some cases on the order of 7/8 of the total time spent fetching the string. Overhead for style is much smaller, but still a substantial portion of total time spent fetching an attributed string.

I propose:

1. Introducing two new attributes -
AXSimpleAttributedStringForTextMarkerRange - includes neither style nor spell checking
AXMisspelledAttributedStringForTextMarkerRange - includes both style and spell checking

2. Modifying the existing property AXAttributedStringForTextMarkerRange to not include spell checking, but retain style attributes.

(AXAttributedStringForRange will be unchanged.)

Spell checking is only relevant in editable nodes. Assistive technology clients can detect if they or their ancestors are editable and choose the appropriate attribute.

Style is often not output via speech or braille and in those cases is wasted overhead. Again assistive technology can make contextual decisions to ask for the correct attribute.

Other approaches considered:
Leaving AXAttributedStringForTextMarkerRange unchanged and introducing AXAttributedStringForTextMarkerRangeWithOptions allowing spell checking, style and future attributes to be toggled on and off. The overhead of boxing and unboxing these parameters outweighed the cost savings of skipping any individual attribute except spelling in most cases.
Comment 1 Radar WebKit Bug Importer 2016-08-10 13:10:05 PDT
<rdar://problem/27791714>
Comment 2 Doug Russell 2016-08-10 13:29:50 PDT
Created attachment 285753 [details]
Patch
Comment 3 Build Bot 2016-08-10 14:18:27 PDT
Comment on attachment 285753 [details]
Patch

Attachment 285753 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1847779

New failing tests:
accessibility/mac/attributed-string/attributed-string-for-range-not-misspelled .html
accessibility/mac/attributed-string/attributed-string-for-range.html
accessibility/mac/attributed-string/misspelled-attributed-string-for-range.html
accessibility/mac/bounds-for-range.html
accessibility/mac/attributed-string/simple-attributed-string-for-range.html
Comment 4 Build Bot 2016-08-10 14:18:31 PDT
Created attachment 285758 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 5 Build Bot 2016-08-10 14:21:31 PDT
Comment on attachment 285753 [details]
Patch

Attachment 285753 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1847789

New failing tests:
accessibility/mac/bounds-for-range.html
Comment 6 Build Bot 2016-08-10 14:21:35 PDT
Created attachment 285759 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2016-08-10 14:30:14 PDT
Comment on attachment 285753 [details]
Patch

Attachment 285753 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1847798

New failing tests:
accessibility/mac/attributed-string/attributed-string-for-range-not-misspelled .html
accessibility/mac/attributed-string/attributed-string-for-range.html
accessibility/mac/attributed-string/misspelled-attributed-string-for-range.html
accessibility/mac/bounds-for-range.html
accessibility/mac/attributed-string/simple-attributed-string-for-range.html
Comment 8 Build Bot 2016-08-10 14:30:17 PDT
Created attachment 285760 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 9 Doug Russell 2016-08-10 15:25:32 PDT
Created attachment 285771 [details]
Patch
Comment 10 Build Bot 2016-08-10 16:05:09 PDT
Comment on attachment 285771 [details]
Patch

Attachment 285771 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1848210

New failing tests:
accessibility/mac/bounds-for-range.html
Comment 11 Build Bot 2016-08-10 16:05:14 PDT
Created attachment 285776 [details]
Archive of layout-test-results from ews100 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 12 Build Bot 2016-08-10 16:16:16 PDT
Comment on attachment 285771 [details]
Patch

Attachment 285771 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1848243

New failing tests:
accessibility/mac/bounds-for-range.html
Comment 13 Build Bot 2016-08-10 16:16:20 PDT
Created attachment 285777 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 14 Build Bot 2016-08-10 16:18:41 PDT
Comment on attachment 285771 [details]
Patch

Attachment 285771 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1848223

New failing tests:
accessibility/mac/bounds-for-range.html
Comment 15 Build Bot 2016-08-10 16:18:45 PDT
Created attachment 285779 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 16 Doug Russell 2016-08-10 21:22:31 PDT
Created attachment 285812 [details]
Patch
Comment 17 Doug Russell 2018-02-26 19:49:59 PST
Created attachment 334666 [details]
Updated patch

only introduce one new attribute. omit parameter for style for now. leave AXAttributedStringForTextMarkerRange behavior intact.
Comment 18 Doug Russell 2018-02-26 20:03:16 PST
Created attachment 334667 [details]
Updated patch
Comment 19 Doug Russell 2018-02-26 20:09:12 PST
Created attachment 334668 [details]
Updated patch

fix atk build
Comment 20 Doug Russell 2018-02-26 20:14:06 PST
Created attachment 334669 [details]
Updated patch

try again
Comment 21 chris fleizach 2018-02-27 12:21:19 PST
Comment on attachment 334669 [details]
Updated patch

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

> Source/WebCore/ChangeLog:28
> +        AX: Spell check and style attributes should be optional when fetching attributed string

duplicate entries in changelog

> Tools/ChangeLog:44
> +        AX: Spell check and style attributes should be optional when fetching attributed string

duplicate entries

> Tools/DumpRenderTree/AccessibilityUIElement.cpp:886
> +    AccessibilityTextMarkerRange* markerRange = 0;

= nullptr

> Tools/DumpRenderTree/AccessibilityUIElement.cpp:896
> +    AccessibilityTextMarkerRange* markerRange = 0;

ditto
Comment 22 Doug Russell 2018-02-27 13:02:13 PST
Created attachment 334702 [details]
Updated patch

Updates based on review
Comment 23 WebKit Commit Bot 2018-02-27 14:57:19 PST
Comment on attachment 334702 [details]
Updated patch

Clearing flags on attachment: 334702

Committed r229080: <https://trac.webkit.org/changeset/229080>
Comment 24 WebKit Commit Bot 2018-02-27 14:57:21 PST
All reviewed patches have been landed.  Closing bug.