<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: WKContentView needs to implement UITextInput methods to make speak selection highlighting work"
href="https://bugs.webkit.org/show_bug.cgi?id=166955#c28">Comment # 28</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - AX: WKContentView needs to implement UITextInput methods to make speak selection highlighting work"
href="https://bugs.webkit.org/show_bug.cgi?id=166955">bug 166955</a>
from <span class="vcard"><a class="email" href="mailto:n_wang@apple.com" title="Nan Wang <n_wang@apple.com>"> <span class="fn">Nan Wang</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=300061&action=diff" name="attach_300061" title="patch">attachment 300061</a> <a href="attachment.cgi?id=300061&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=300061&action=review">https://bugs.webkit.org/attachment.cgi?id=300061&action=review</a>
<span class="quote">>> Source/WebCore/dom/Range.h:128
>> + WEBCORE_EXPORT int collectSelectionRectsWithoutUnionInteriorLines(Vector<SelectionRect>&);
>
> Seems like line number should be unsigned rather than int.</span >
The SelectionRect::lineNumber() is int and all the comparisons are int within the function. So I'd rather keep it rather than doing the casts.
<span class="quote">>> Source/WebCore/editing/TextIterator.cpp:2681
>> +static Ref<Range> findPlainTextMatchesInRange(const Range& range, const String& target, FindOptions options, size_t& matchStart, size_t& matchLength, const std::function<bool(size_t, size_t)>& matches)
>
> I find the design of this function quite peculiar and not straightforward as it should be. The idea that the function’s job is to fill in the two values, matchStart and matchLength, as a side effect, is super-strange. Can we find a more straightforward design pattern?</span >
Yes, I agree this looks kinda strange. The problem is that based on Ryosuke's previous comment, we want to hold matchStart and matchLength inside the lambda but we have to use the two values once the searching is done, which can only be in the first function that passes in this lambda. I'll try to break this function and make things more straightforward. If it's still not clear, I think I'd revert it to my original design.</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>