[Webkit-unassigned] [Bug 122644] [ATK] Simplify implementation of atk_text_get_text
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 11 10:13:11 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=122644
Mario Sanchez Prada <mario at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #213978|0 |1
is obsolete| |
Attachment #213978|review? |
Flag| |
Attachment #213996| |review?
Flag| |
--- Comment #3 from Mario Sanchez Prada <mario at webkit.org> 2013-10-11 10:11:59 PST ---
Created an attachment (id=213996)
--> (https://bugs.webkit.org/attachment.cgi?id=213996&action=review)
New patch proposal
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
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list