Thanks!
On Thu, Sep 18, 2008 at 4:13 PM, John LaBanca <[EMAIL PROTECTED]> wrote:
> 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"
>>
>
>
--
"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
-~----------~----~----~----~------~----~------~--~---
