[webkit-reviews] review canceled: [Bug 122644] [ATK] Simplify implementation of atk_text_get_text : [Attachment 213978] Patch proposal

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <mario at webkit.org> has canceled Mario Sanchez Prada
<mario at webkit.org>'s request for review:
Bug 122644: [ATK] Simplify implementation of atk_text_get_text
https://bugs.webkit.org/show_bug.cgi?id=122644

Attachment 213978: Patch proposal
https://bugs.webkit.org/attachment.cgi?id=213978&action=review

------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
I just realized that the previous patch had an issue: when calling
doAXStringForRange() from the AtkText implementation, it's very common that we
pass (0, -1) as start/length offset, which is something doAXStringForRange()
won't handle:

  String AccessibilityRenderObject::doAXStringForRange(const PlainTextRange&
range) const
  {
     [...]
     String elementText = isPasswordField() ? passwordFieldValue() : text();
     if (range.start + range.length > elementText.length())
	 return String();
    
     return elementText.substring(range.start, range.length);
  }

However, I don't see any reason why doAXStringForRange() could not handle a
(start, -1) range, since String::substring() would gracefully handle that
situation in case we passed a too big length as a second parameter, or just -1
(which would be translated to an unsigned). Besides, it seems to me that the
check (range.start + range.length > elementText.length()) might be wrong
anyway, or at least I don't understand very well what it's trying to check.

For all those reasons, the current patch proposes to remove that if and let
String::substring() handle the out-of-bounds scenario when passing start/end