<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: richer text change notifications"
href="https://bugs.webkit.org/show_bug.cgi?id=142719#c110">Comment # 110</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: richer text change notifications"
href="https://bugs.webkit.org/show_bug.cgi?id=142719">bug 142719</a>
from <span class="vcard"><a class="email" href="mailto:bfulgham@webkit.org" title="Brent Fulgham <bfulgham@webkit.org>"> <span class="fn">Brent Fulgham</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=251541&action=diff" name="attach_251541" title="Fix windows builds">attachment 251541</a> <a href="attachment.cgi?id=251541&action=edit" title="Fix windows builds">[details]</a></span>
Fix windows builds
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=251541&action=review">https://bugs.webkit.org/attachment.cgi?id=251541&action=review</a>
<span class="quote">> LayoutTests/ChangeLog:8
> + Richer accessibility value change notifications. Introduce AXTextEditType, postTextStateChangeNotification and postTextReplacementNotification to give assistive tech apps more reliable context for responding to changes in web content. Also implement a mechanism to post value changes in password form fields in coalesced ticks to thwart analyzing the cadence of changes.</span >
Nit: These lines are kind of long. We usually hard-return at around 80 characters or so since many of our tools don't do auto-line-wrap.
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:872
> +void AXObjectCache::showIntent(const AXTextStateChangeIntent &intent)</span >
Your using objc syntax here. Should be "const AXTextStateChangeIntent& intent)
<span class="quote">> Source/WebCore/accessibility/AXObjectCache.cpp:985
> +void AXObjectCache::setTextSelectionIntent(AXTextStateChangeIntent intent)</span >
Why is this passed by value here, when you pass by reference in the above method? If AXTextStateChangeIntent is non-trivial in size, we should be reference passing it everywhere.
<span class="quote">> Source/WebCore/editing/Editor.cpp:2851
> +void Editor::changeSelectionAfterCommand(const VisibleSelection& newSelection, FrameSelection::SetSelectionOptions options, AXTextStateChangeIntent intent)</span >
Seems like AXTextStateChangeIntent should be passed by reference, since it's an object?
<span class="quote">> Source/WebCore/editing/FrameSelection.cpp:331
> +void FrameSelection::setSelection(const VisibleSelection& selection, SetSelectionOptions options, AXTextStateChangeIntent intent, CursorAlignOnScroll align, TextGranularity granularity)</span >
Seems like AXTextStateChangeIntent should be passed as a reference.
<span class="quote">> Source/WebCore/editing/FrameSelection.cpp:371
> +void FrameSelection::updateAndRevealSelection(AXTextStateChangeIntent intent)</span >
const AXTextStateChangeIntent& ?
<span class="quote">> Source/WebCore/editing/FrameSelection.cpp:1036
> + AXTextStateChangeIntent intent = AXTextStateChangeIntent();</span >
I think this could just be "AXTextStateChangeIntent intent;"
<span class="quote">> Source/WebCore/editing/FrameSelection.h:148
> + WEBCORE_EXPORT void setSelection(const VisibleSelection&, SetSelectionOptions = defaultSetSelectionOptions(), AXTextStateChangeIntent = AXTextStateChangeIntent(), CursorAlignOnScroll = AlignCursorOnScrollIfNeeded, TextGranularity = CharacterGranularity);</span >
These AXTextStateChangeIntent seem like they should be references.
<span class="quote">> Source/WebCore/editing/FrameSelection.h:276
> + void updateAndRevealSelection(AXTextStateChangeIntent = AXTextStateChangeIntent());</span >
Ditto.
<span class="quote">> Source/WebCore/editing/FrameSelection.h:303
> + void notifyAccessibilityForSelectionChange(AXTextStateChangeIntent = AXTextStateChangeIntent());</span >
Ditto.
<span class="quote">> Source/WebCore/editing/FrameSelection.h:305
> + void notifyAccessibilityForSelectionChange(AXTextSelectionIntent) { }</span >
Ditto.
<span class="quote">> Source/WebCore/editing/FrameSelection.h:372
> +inline void FrameSelection::notifyAccessibilityForSelectionChange(AXTextStateChangeIntent)</span >
Ditto.
<span class="quote">> Source/WebCore/editing/atk/FrameSelectionAtk.cpp:81
> +void FrameSelection::notifyAccessibilityForSelectionChange(AXTextStateChangeIntent)</span >
const AXTextStateChangeIntent& ?
<span class="quote">> Source/WebCore/editing/mac/FrameSelectionMac.mm:50
> +void FrameSelection::notifyAccessibilityForSelectionChange(AXTextStateChangeIntent intent)</span >
const AXTextStateChangeIntent& ?</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>