Bug 238222 - [DataBinding] Round-tripping converted values
Summary: [DataBinding] Round-tripping converted values
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.4   Edit
Hardware: All All
: P3 enhancement with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 126918 194826 208304 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-06-24 06:01 EDT by Christoph Jäger CLA
Modified: 2021-12-29 15:40 EST (History)
12 users (show)

See Also:


Attachments
The ValidatorRoundtrip class (1.35 KB, text/plain)
2009-04-12 13:27 EDT, Sylvere Richard CLA
no flags Details
Snippet using ValidatorRoundtrip class (6.04 KB, text/plain)
2009-04-12 13:28 EDT, Sylvere Richard CLA
no flags Details
demonstrate the problem using an overriden updatevaluestrategy (5.94 KB, text/plain)
2009-04-19 13:00 EDT, Sylvere Richard CLA
no flags Details
overridden updatevaluestrategy #2 (6.94 KB, text/plain)
2009-04-22 14:05 EDT, Sylvere Richard CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Jäger CLA 2008-06-24 06:01:06 EDT
When using a custom converter when binding a Text field to a String property (for instance a converter to trim whitespace from the value the user entered), the converted value is propagated from the Text field to the String property (target -> converter -> converted value -> model), but the content of the Text field stays the same, in this example the whitespace is still there.

In most cases, the desired behaviour would probably be that the model and target are syncronized, meaning the Text field always shows the value of the String property (possibly including conversion from model to target).

Right now, the only solution seems to be to manually update the target using Binding#updateModelToTarget(). This can be achieved by extending UpdateValueStrategy to override:

--
private Binding binding;

public void setBinding(Binding binding)
{
    this.binding=binding;
}

@Override
protected IStatus doSet(IObservableValue observableValue, Object value)
{
    IStatus status=super.doSet(observableValue, value);
    if (binding!=null) binding.updateModelToTarget();
    return status;
}
--

and setting the binding:

targetToModelStrategy.setBinding(binding);

But this seems to be an ugly workaround. I think the best solution would be to automatically or by setting some flag synchronize the target with the model.
Comment 1 Matthew Hall CLA 2008-06-24 15:45:44 EDT
I dub this feature "round-tripping."  :)

I think the best place for this feature is in the update strategy.  This could take the form of a new chained method call, e.g.:

dbc.bindValue(target, model, new UpdateValueStrategy().setRoundTrip(true), null);

or as a new update policy constant, e.g.:

dbc.bindValue(target, model,
    new UpdateValueStrategy(UpdateValueStrategy.POLICY_ROUND_TRIP), null);

A round trip would be about the same as executing doUpdate() as normal, and then calling it again with the source and target observables reversed.

One problem we may run into is the effect that round tripping will have on the caret position in text controls.  If the round-trip value trims or eliminates characters, the caret position after the update could be semantically different to where it was before the update.  This has the potential to be just as jarring and annoying as bumping the touchpad while typing.  At the least we should avoid updating the observable whenever the new value is the same as the current value.
Comment 2 Boris Bokowski CLA 2008-07-31 16:19:39 EDT
*** Bug 126918 has been marked as a duplicate of this bug. ***
Comment 3 Boris Bokowski CLA 2008-07-31 16:20:41 EDT
*** Bug 194826 has been marked as a duplicate of this bug. ***
Comment 4 Boris Bokowski CLA 2008-07-31 16:21:55 EDT
*** Bug 208304 has been marked as a duplicate of this bug. ***
Comment 5 Boris Bokowski CLA 2008-07-31 16:33:02 EDT
From bug 194826 comment 2:
"I'm curious how you see this working in regards to the user experience. 
If you allow the user to enter data, the control displays it, format the text,
and then update the control you have the potential of the cursor jumping
around, losing selection, etc. while the user is entering text."

From bug 208304 comment 3:
"The question is how this interacts with what the user is doing. For
example, you should not change the contents of a text field while the keyboard
focus is still in that text field. (Unless you are very careful and handle all
the different corner cases well.)"

From bug 208304 comment 5:
"Have you looked at the FormattedText custom control from
http://www.eclipse.org/nebula ?"

It would be good to collect example SWT snippets that implement masking and/or formatting in text fields, as well as examples how other apps deal with this concern.
Comment 6 Dave Orme CLA 2008-08-01 15:15:54 EDT
(In reply to comment #5)
> From bug 208304 comment 5:
> "Have you looked at the FormattedText custom control from
> http://www.eclipse.org/nebula ?"

This is the approach I currently recommend.

Round tripping seems like a worthwhile idea too.
Comment 7 Matthew Hall CLA 2008-08-04 14:50:05 EDT
Round trip API cannot be put into the UpdateStrategy: in a target-to-model update, the model-to-target return trip must be routed through the other update strategy.  We may have to put this API into the Binding class.  This poses a problem because there is no callback from the update strategy indicating when the update pipeline has completed, in order to initiate the return trip.
Comment 8 Sylvere Richard CLA 2009-04-12 13:25:51 EDT
I'm facing a similar problem.
My user wants to restore the last valid value in case an error occurs during validation.
If my AfterGetValidator returns an IStatus.ERRROR, the wrong value is still displayed on screen...

So I decided to use the "ugly workaround" of Christoph Jäger by overriding the doSet method of UpdateValueStrategy:

@Override
protected IStatus doSet(IObservableValue observableValue, Object value)
{
    IStatus status=super.doSet(observableValue, value);
    if (binding!=null) binding.updateModelToTarget();
    return status;
}


Unfortunately, this solution does not work for us all the time.
If a user enters 'abc', which causes an error, the overrided method will reinit to the last valid value => ok
If the user re-enters 'abc', the overrided method will never be called because the framework thinks nothing has changed since the last time. So the bad value is still displayed => ko

So I tried to find another solution. For now, the solution we use is based on the following ValidatorRoundtrip class :

public abstract class ValidatorRoundtrip implements IValidator {

	private final Binding b;
	private final Realm realm;

	public ValidatorRoundtrip(Realm realm, Binding b) {
		this.realm = realm;
		this.b = b;
	}

	/**
	 * Method declared final. Validation logic must be put in method 
	 * ValidatorRoundtrip.doValidation
	 * 
	 * 
	 * Will call doValidation. 
	 * 
	 * If the IStatus returned by doValidation is an error
	 * or a cancel, it will force a refresh of the target by calling updateModelToTarget()
	 * to restore the previous value on screen.
	 * Otherwise, the process flow is unchanged.
	 */
	@Override
	public final IStatus validate(Object value) {
		final IStatus res = doValidation(value);
		if (res.matches(IStatus.INFO | IStatus.WARNING) || res.isOK()) {
			return res;
		}

		realm.asyncExec(new Runnable() {

			@Override
			public void run() {
				b.updateModelToTarget();
			}
		});
		return res;
	}

	/**
	 * Puts validation logic here
	 * @param value
	 * @return 
	 */
	public abstract IStatus doValidation(Object value);
}

This time, whenever an error occurs, the screen keeps in synch with its model.
I submitted a snippet demonstrating how this class works.

In this snippet, a text is observed with a SWT.FocusOut trigger (SWT.Modify would have no sense here since we cannot reinit a text in the same time the user is modifying it...).
Each time the text loses focus, the validation occurs and restores the previous value or displays a warning if necessary.

I don't know if a better solution exist. 
I'm really interested in any other better alternatives.
So, comments are welcomed!!

Comment 9 Sylvere Richard CLA 2009-04-12 13:27:24 EDT
Created attachment 131571 [details]
The ValidatorRoundtrip class
Comment 10 Sylvere Richard CLA 2009-04-12 13:28:38 EDT
Created attachment 131572 [details]
Snippet using ValidatorRoundtrip class
Comment 11 Matthew Hall CLA 2009-04-17 21:34:41 EDT
> If a user enters 'abc', which causes an error, the overrided method will
> reinit to the last valid value => ok
> If the user re-enters 'abc', the overrided method will never be called
> because the framework thinks nothing has changed since the last time.
> So the bad value is still displayed => ko

Could you provide a snippet that demonstrates this?  I would expect the framework to fire a change event both times the user enters 'abc' so I'd like to see this in action.
Comment 12 Sylvere Richard CLA 2009-04-19 13:00:52 EDT
Created attachment 132350 [details]
demonstrate the problem using an overriden updatevaluestrategy

Hi Matthew,

please find as attachment the requested snippet.
It contains only a text box and button.
The text box must reject text containing the sequence 'aa' and reset to the last valid value before the user enters a wrong value.

Here is the "modus operandi" to see the problem with this solution base on an overriden UpdateValueStrategy

1/ enter 'abc' -> the value is correct, so the model contains now 'abc' 
=> ok

2/ enter 'abcaa' in the text box -> the value is incorrect, but thanks to the overriden UpdateValueStrategy, the text box displays the last valid value 'abc'. The roundtrip works !
=> ok

3/ re-enter 'abcaa' in the text box -> the value is incorrect but the roundtrip does not occur !!! The string 'abcaa' is still displayed in the text box.
=> ko

I hope this snippet corresponds to what you asked me for.

Because of this problem, we use now the solution described in the snippet using the validator roundtrip.
Comment 13 Matthew Hall CLA 2009-04-22 12:56:29 EDT
I took a look at this last week and thought I responded but I guess I didn't.  I don't have time right now to look at this again thoroughly but I think part of the problem was that you are not implementing property change support in your class per the bean specification.  Add the following to your bean class (many people put these in a base ModelObject class in their projects and then extend their model classes from that base class):

  private changeSupport = new PropertyChangeSupport(this);

  public void addPropertyChangeListener(PropertyChangeListener listener) {
    changeSupport.addPropertyChangeListener(listener);
  }

  public void removePropertyChangeListener(PropertyChangeListener listener) {
    changeSupport.removePropertyChangeListener(listener);
  }

  public void addPropertyChangeListener(String property,
      PropertyChangeListener listener) {
    changeSupport.addPropertyChangeListener(property, listener);
  }

  public void removePropertyChangeListener(String property,
      PropertyChangeListener listener) {
    changeSupport.removePropertyChangeListener(property, listener);
  }

  protected void firePropertyChange(String propertyName, Object oldValue,
      Object newValue) {
    changeSupport.firePropertyChange(propertyName, oldValue, newValue);
  }

Then you need to update your setters to fire a property change as follows:

  public void setMyText(String myText) {
    firePropertyChange("myText", this.myText, this.myText = myText);
  }
Comment 14 Sylvere Richard CLA 2009-04-22 14:05:49 EDT
Created attachment 132819 [details]
 overridden updatevaluestrategy  #2

I have updated the snippet to add a PropertyChangeListener to MyModel as you suggest. 

Moreover, in the previous version of the snippet, I used a PojoObservable to create an IObservableValue on the property "myText" of the model "MyModel". In the new version of the snippet, I use now the BeansObservables factory to create the IObservable. 

The result is not better, the roundtrip does not occur the second time we enter the same wrong value.

I note that you don't have time to investigate for now. It's ok for us since we have a solution that seems to work with the ValidatorRoundtrip class.

I tried to use the debugger to find why the text box is not updated the second time. The text box is wrapped in a TextObservableValue that fires events when the text change. This class has property called oldValue that contains the previous value of the text box. It seems this field is not updated by the call

binding.updateModelToTarget();

made in the method IStatus doSet(IObservableValue observableValue, Object value) we override in UpdateValueStrategy.

So since the newValue is the same as the oldValue in the TextObservableValue, no event is fired and the roundtrip does not occur.
Comment 15 Matthew Hall CLA 2009-04-27 13:24:00 EDT
Sylvere, when I run your latest attachment, I see the text field getting reset every time and not just the first time as you described.  Am I missing something?
Comment 16 Sylvere Richard CLA 2009-04-28 12:34:52 EDT
(In reply to comment #15)
> Sylvere, when I run your latest attachment, I see the text field getting reset
> every time and not just the first time as you described.  Am I missing
> something?
> 

Matthew,
thank you for testing my last snippet.
I don't really understand why you can't reproduce the problem with the last snippet.
When I run it and follow the procedure I described in my previous comments, namely :

"steps to reproduce the bug :
1/ enter 'abc' -> the value is correct, so the model contains now 'abc' 
=> ok

2/ enter 'abcaa' in the text box -> the value is incorrect, but thanks to the
overriden UpdateValueStrategy, the text box displays the last valid value
'abc'. The roundtrip works !
=> ok

3/ re-enter 'abcaa' in the text box -> the value is incorrect but the roundtrip
does not occur !!! The string 'abcaa' is still displayed in the text box.
=> ko"

I can reproduce the problem each time...

I run this snippet on Windows Vista + JRE1.6.0 upd11
I tested this snippet too on a windows XP + JRE1.4.2 upd18 with the same result.

My classpath contains the following jars :
com.ibm.icu_3.8.1.v20080530.jar
org.eclipse.core.commands_3.4.0.I20080509-2000.jar
org.eclipse.core.databinding_1.1.1.M20080827-0800b.jar
org.eclipse.core.databinding.beans_1.1.1.M20080827-0800a.jar
org.eclipse.core.runtime_3.4.0.v20080512.jar
org.eclipse.equinox.common_3.4.0.v20080421-2006.jar
org.eclipse.jface_3.4.2.M20090107-0800.jar
org.eclipse.jface.databinding_1.2.1.M20080827-0800a.jar
org.eclipse.jface.source_3.4.2.M20090107-0800.jar
org.eclipse.osgi_3.4.3.R34x_v20081215-1030.jar
org.eclipse.swt_3.4.2.v3452b.jar
org.eclipse.swt.win32.win32.x86_3.4.1.v3452b.jar

I can't figure out why the snippet's behavior is different on your system(s)...