Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 195171 - [Text] Implement selection methods for Text with SWT.MULTI
Summary: [Text] Implement selection methods for Text with SWT.MULTI
Status: RESOLVED FIXED
Alias: None
Product: RAP
Classification: RT
Component: RWT (show other bugs)
Version: 1.0   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 1.1.1   Edit
Assignee: Rüdiger Herrmann CLA
QA Contact:
URL:
Whiteboard: qx-closed
Keywords:
Depends on:
Blocks:
 
Reported: 2007-07-02 14:58 EDT by Rüdiger Herrmann CLA
Modified: 2008-09-24 05:50 EDT (History)
2 users (show)

See Also:


Attachments
Patch to enable the text-selection in multi-line text widgets (6.28 KB, patch)
2008-07-29 09:38 EDT, Christian Janz CLA
jkrause: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rüdiger Herrmann CLA 2007-07-02 14:58:17 EDT
Even though API to get and set selection exists for the Text widget, the LCA for multi-line text widgets does not implement it yet.
This depends on qooxdoo bug #521 (http://bugzilla.qooxdoo.org/show_bug.cgi?id=521)
Comment 1 Christian Janz CLA 2008-07-29 09:36:46 EDT
I made a proposal for a fix of this bug that allows to get and set the selection for multi-line text widgets. The patch was tested with FF2, FF3 and IE7.
Comment 2 Christian Janz CLA 2008-07-29 09:38:03 EDT
Created attachment 108641 [details]
Patch to enable the text-selection in multi-line text widgets
Comment 3 Ralf Sternberg CLA 2008-08-06 10:18:08 EDT
(In reply to comment #2)
Christian, thanks for the patch. For me it seems that the change to TextArea.js is no longer neccessary, since we are on qx-0.7.3 now and the bug mentioned above has been fixed. Can you confirm this?

There were two changes in your patch in MultiTextDisplayLCA which I did not apply because I was unsure about their intention and side-effects. Could you please explain why you made these changes? Maybe you're right and I just don't see it.

There seems to be another issue with MULTI Texts in IE7, which results in a selected region that is shifted from the setSelection arguments by 1 character if the first argument is greater that 0. However, this problem also exists with your patch. I re-enabled the selection getter and setter anyway to make it easier to track down this bug. Please reopen if there are still problems.
Comment 4 Christian Janz CLA 2008-08-07 05:09:31 EDT
Hi Ralf, thanks for your comment. You're right: with qx-0.7.3, patching the selection methods of TextArea in TextUtil.js is no longer neccessary. I tested the selection in TextAreas in FF2/3 and IE7 with the newest RAP version (without my replaced selection methods) and it worked fine.

In MultiTextLCA I only re-enabled writing the selection via "TextLCAUtil.writeSelection( text )" in renderChanges(). So I think you mean my changes in TextLCAUtil. In the following I will give a comment on my changes:

- I moved the "text.setSelection( selection );" statement in readTextAndSelection( final Text text ) from line 102 to 104: The problem was that reading the selection was coupled to reading the text. If the text value of a TextField or TextArea hasn't changed on JS side it isn't sent with the request, so the request-property "text" is null. In this case the selection is read from the request correctly in line 78, but as the text value is null the selection isn't set to text Java Text widget. By moving the setSelection call out of the if(txt!=null) block the selection is set regardless of the text value.

- In readSelection( final Text text ) I removed the if( ( text.getStyle() & SWT.MULTI ) == 0 )  check. Because reading/writing of selection is now enabled in multi-line text widgets this check has to be removed.

- In writeSelection( final Text text ) on line 181, I changed the condition to if( WidgetLCAUtil.hasChanged( text, PROP_SELECTION, newValue, defValue ) ||  WidgetLCAUtil.hasChanged( text, PROP_TEXT, text.getText(), "" )): This is neccessary because if the text value changed, the selection is reset to (0, 0) on JS-side (Rüdiger made  a comment on this in line 178). So the selection set in Java isn't applied on the JS-side. E.g. the selection in Java is (1, 2) and didn't change during the request; the text changed from "test" to "test2". After rendering the selection of the text widget in the browser is (0, 0) but should be (1, 2).

I could reproduce the bug concerning the shifted selection in IE7, too. 
I think you can reopen this bug (I can't because I'm not the reporter :-) so that you can decide if you want to apply my changes in TextLCAUtil and check the selection problem in IE7 (perhaps qooxdoo bug #521 should be reopened because of this issue, too).
Comment 5 Ralf Sternberg CLA 2008-08-12 09:03:16 EDT
(In reply to comment #4)
Hi Christian,

thanks for your detailed description.

> In MultiTextLCA I only re-enabled writing the selection via
> "TextLCAUtil.writeSelection( text )" in renderChanges(). So I think you mean my
> changes in TextLCAUtil. In the following I will give a comment on my changes:

You're right, I meant TextLCAUtil.

> - I moved the "text.setSelection( selection );" statement in
> readTextAndSelection( final Text text ) from line 102 to 104: The problem was
> that reading the selection was coupled to reading the text. If the text value of
> a TextField or TextArea hasn't changed on JS side it isn't sent with the
> request, so the request-property "text" is null. In this case the selection is
> read from the request correctly in line 78, but as the text value is null the
> selection isn't set to text Java Text widget. By moving the setSelection call
> out of the if(txt!=null) block the selection is set regardless of the text
> value.

Right, but I had to change the implementation to handle the case that a verify listener rejects a modification. In this case, the selection must remain unchanged.

> - In readSelection( final Text text ) I removed the if( ( text.getStyle() &
> SWT.MULTI ) == 0 )  check. Because reading/writing of selection is now enabled
> in multi-line text widgets this check has to be removed.

Yes, sure. This change is already in the CVS.

> - In writeSelection( final Text text ) on line 181, I changed the condition to
> if( WidgetLCAUtil.hasChanged( text, PROP_SELECTION, newValue, defValue ) ||
> WidgetLCAUtil.hasChanged( text, PROP_TEXT, text.getText(), "" )): This is
> neccessary because if the text value changed, the selection is reset to (0, 0)
> on JS-side (Rüdiger made  a comment on this in line 178). So the selection set
> in Java isn't applied on the JS-side. E.g. the selection in Java is (1, 2) and
> didn't change during the request; the text changed from "test" to "test2". After
> rendering the selection of the text widget in the browser is (0, 0) but should
> be (1, 2).

I disagree. When changing the text, the selection is set to (0,0) on the server side, too. Thus, in your example, it should be (0,0) instead of (1,2).

> I could reproduce the bug concerning the shifted selection in IE7, too.
> I think you can reopen this bug (I can't because I'm not the reporter :-) so
> that you can decide if you want to apply my changes in TextLCAUtil and check the
> selection problem in IE7 (perhaps qooxdoo bug #521 should be reopened because of
> this issue, too).

I think the shifted selection in IE is a bug of its own. I coudn't reproduce it anymore, but if you have a snippet that reproduces it, could you please open up another bug?