[Webkit-unassigned] [Bug 122644] [ATK] Simplify implementation of atk_text_get_text

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Mario Sanchez Prada <mario at webkit.org>  2013-10-17 08:35:53 PST ---
(In reply to comment #6)
> [...]
> > 1. Because I think it's checking the wrong thing, since a proper check to see if the range is out of bounds would be something like this, IMO:
> > 
> >    (range.length - range.start > elementText.length())
> 
> I think that's wrong. I think we still want to check the MAXRange of that 
> range, which would be +.

Stupid me. I wast reading that line as if was using the final position ("range.end", if it existed) instead of the length.

You are actually checking that the range of text does not fall out of the limits of the text control, hence the original check is semantically right, not mine.

> > 
> > 2. Because in any case the String::substring() method will check, and adjust if needed, any limit that falls out of bounds for that string.
> 
> If it in fact does do this, then it's OK to remove. I was assuming that
> would throw an error
> 

This is the implementation of substring():

  PassRefPtr<StringImpl> StringImpl::substring(unsigned start, unsigned length)
  {
    if (start >= m_length)
        return empty();
    unsigned maxLength = m_length - start;
    if (length >= maxLength) {
        if (!start)
            return this;
        length = maxLength;
    }
    if (is8Bit())
        return create(m_data8 + start, length);

    return create(m_data16 + start, length);
  }


AS far as I can see, if the length passed is bigger than the String's upper limit, this implementation will just adjust it to maxLength, so I think it should be fine to remove that check anyway.

What do you think?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.