committed as r3666
I see what you are saying about the more efficient version, so I changed it
to the following where we don't get the length unless we need to:
>
> public String getSelectedText() {
>
int start = getCursorPos();
>
if (start < 0) {
>
return "";
>
}
>
int length = getSelectionLength();
>
return getText().substring(start, start + length);
>
}
>
Thanks,
John LaBanca
[EMAIL PROTECTED]
On Thu, Sep 18, 2008 at 3:48 PM, Emily Crutcher <[EMAIL PROTECTED]> wrote:
> I'm not entirely sure I understand why you can't use the slightly more
> efficient check in getSelectedText(), but as that is only a speed
> optimization , LGTM.
>
>
>
> On Thu, Sep 18, 2008 at 3:40 PM, John LaBanca <[EMAIL PROTECTED]> wrote:
>
>> IE throws an exception if the cursorPos is negative, whereas the other
>> browsers return the correct cursor position. I updated the JavaDoc of
>> setSelectionRange() and the two methods that use it (selectAll and
>> setCursorPos) to indicate that they can only be used when the widget is
>> attached and visible. Previously, they just said that the widget had to be
>> attached.
>> Thanks,
>> John LaBanca
>> [EMAIL PROTECTED]
>>
>>
>> On Thu, Sep 18, 2008 at 3:20 PM, Emily Crutcher <[EMAIL PROTECTED]> wrote:
>>
>>> That makes sense, can we add some javadoc to the methods to let people
>>> know they shouldn't call this method when the text box is hidden? As right
>>> now we claim it will work as long as the field is attached to the document.
>>>
>>> For the IE case, will IE ever return anything but 0 for the selection
>>> length if the start is -1? If so, it seems like writing the check in terms
>>> of selection length might be more efficient in the general case, something
>>> like this psuedo code:
>>>
>>> public String getSelectedText() {
>>> int length = getSelectionLength();
>>> if (length == 0) {
>>> return "";
>>> }
>>> int start = getCursorPos(),
>>> return getText().substring(start, start + length);
>>>
>>> }
>>> On Thu, Sep 18, 2008 at 2:54 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>>
>>>> The problem is, we can't assert that the element isn't visible because
>>>> that isn't an efficient or reliable thing to check, and we don't really
>>>> want
>>>> to throw an exception at runtime in FF if somebody is developing in IE and
>>>> may not test FF thoroughly. So, I think its better to fail cleanly. If
>>>> you
>>>> notice the other methods getCursorPos() and getSelectionLength(), we
>>>> already
>>>> silently fail those cases for FF.
>>>> Thanks,
>>>> John LaBanca
>>>> [EMAIL PROTECTED]
>>>>
>>>>
>>>> On Thu, Sep 18, 2008 at 2:44 PM, Emily Crutcher <[EMAIL PROTECTED]> wrote:
>>>>
>>>>> The setSelectionRange() method silently failing seems like a bad idea
>>>>> to me. If we cannot support it on Firefox, wouldn't it be better to alert
>>>>> the user of that with a java exception?
>>>>>
>>>>>
>>>>>
>>>>> On Thu, Sep 18, 2008 at 2:43 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>>>>
>>>>>> +gwtContrib
>>>>>> Thanks,
>>>>>> John LaBanca
>>>>>> [EMAIL PROTECTED]
>>>>>>
>>>>>>
>>>>>> On Thu, Sep 18, 2008 at 2:21 PM, John LaBanca <[EMAIL PROTECTED]>wrote:
>>>>>>
>>>>>>> Emily -
>>>>>>> Please do a code review on this two-for patch for TextBox.
>>>>>>>
>>>>>>> Description:
>>>>>>> ========
>>>>>>> Calling TextBoxBase.setSelectionRange(elem, int, int) throws an
>>>>>>> exception in FF if the element is either unattached or not visible (ie.
>>>>>>> display:none). Also, getSelectedText() can throw an exception if the
>>>>>>> cursorPos is reported as -1, which it can be in IE.
>>>>>>>
>>>>>>> Fix:
>>>>>>> ====
>>>>>>> TextBoxBase.setSelectionRange() now checks if an exception occurs and
>>>>>>> ignores it. getSelectedText() now checks if the cursorPos is -1.
>>>>>>>
>>>>>>> Testing:
>>>>>>> ======
>>>>>>> Created unit test for the setSelectionRage() case, and verified the
>>>>>>> following methods on all main browsers (IE versions of them):
>>>>>>> selectAll()
>>>>>>> getSelectionLength()
>>>>>>> getSelectedText()
>>>>>>> getCursorPos()
>>>>>>> setCursorPos()
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> John LaBanca
>>>>>>> [EMAIL PROTECTED]
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> "There are only 10 types of people in the world: Those who understand
>>>>> binary, and those who don't"
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> "There are only 10 types of people in the world: Those who understand
>>> binary, and those who don't"
>>>
>>
>>
>
>
> --
> "There are only 10 types of people in the world: Those who understand
> binary, and those who don't"
>
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
