[Webkit-unassigned] [Bug 166955] AX: WKContentView needs to implement UITextInput methods to make speak selection highlighting work

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=166955

--- Comment #28 from Nan Wang <n_wang at apple.com> ---
Comment on attachment 300061
  --> https://bugs.webkit.org/attachment.cgi?id=300061
patch

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

>> Source/WebCore/dom/Range.h:128
>> +    WEBCORE_EXPORT int collectSelectionRectsWithoutUnionInteriorLines(Vector<SelectionRect>&);
> 
> Seems like line number should be unsigned rather than int.

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.

>> 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?

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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170129/73cc0e07/attachment.html>