[webkit-reviews] review granted: [Bug 267477] AX: Add AccessibilityThreadTextApisEnabled flag and hook it up to several APIs : [Attachment 469387] Patch

[bugzilla-daemon at webkit.org]

chris fleizach <cfleizach at apple.com> has granted  review:
Bug 267477: AX: Add AccessibilityThreadTextApisEnabled flag and hook it up to
several APIs
https://bugs.webkit.org/show_bug.cgi?id=267477

Attachment 469387: Patch

https://bugs.webkit.org/attachment.cgi?id=469387&action=review




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

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

>>>
LayoutTests/accessibility/ax-thread-text-apis/text-marker-with-user-select-none
-expected.txt:3
>>> +FAIL: textElement.textMarkerRangeLength(textMarkerRange) !== 43, was 39
>> 
>> Should we be committing these with the expected PASS value, rather than
FAIL? I see you have a FIXME below, but I wonder if it's better to get a
failure when running the test, rather than a false positive.
> 
> That's a good question. I chose to commit FAIL to try to keep our post-commit
bot results clean...but thinking about it more, we're the only ones watching
that, and it seems pretty logical to just ignore failures in the
ax-thread-text-apis/ folder. Curious to hear what others think, but I think
committing the expected PASS (and letting the test fail) instead could make
sense.

Is the strategy to fix this before enabling feature? if so, probably ok to
commit expected results (PASS) and then when it gets enabled and not skipped,
we ensure it passes. otherwise easy to forget a task like this