This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 9631 - Entering incorrect value into property sheet is not user friendly
Summary: Entering incorrect value into property sheet is not user friendly
Status: CLOSED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Window System (show other bugs)
Version: 3.x
Hardware: All All
: P3 major (vote)
Assignee: David Strupl
URL:
Keywords:
: 8413 (view as bug list)
Depends on: 11558
Blocks:
  Show dependency tree
 
Reported: 2001-02-15 00:18 UTC by Rochelle Raccah
Modified: 2008-12-22 20:31 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rochelle Raccah 2001-02-15 00:18:52 UTC
In my property editor, in setText, if the value is illegal, I show an alert
dialog with the error.  Now it comes up twice if the value is illegal.  I added
a Thread.dumpStack() to my code and found PropertyDisplayer applying the changes
twice: once in actionPerformed and once in focusLost.  For a user, it's quite
annoying to get the error twice, but in general applying the value twice (even
if it is legal) is a bad thing.

1) First time (excerpt):
java.lang.Exception: Stack trace
        at java.lang.Thread.dumpStack(Thread.java:993)
        at
com.sun.forte4j.persistence.internal.ui.modules.mapping.nodes.ClassFilterNode$5.setAsText(ClassFilterNode.java:641)
        at
org.openide.explorer.propertysheet.PropertyDisplayer.setAsText(PropertyDisplayer.java:729)
        at
org.openide.explorer.propertysheet.PropertyDisplayer$7.actionPerformed(PropertyDisplayer.java:616)
        at javax.swing.JTextField.fireActionPerformed(JTextField.java:421)
        at javax.swing.JTextField.postActionEvent(JTextField.java:586)
        at
javax.swing.JTextField$NotifyAction.actionPerformed(JTextField.java:696)

2) Second time (excerpt):
java.lang.Exception: Stack trace
        at java.lang.Thread.dumpStack(Thread.java:993)
        at
com.sun.forte4j.persistence.internal.ui.modules.mapping.nodes.ClassFilterNode$5.setAsText(ClassFilterNode.java:641)
        at
org.openide.explorer.propertysheet.PropertyDisplayer.setAsText(PropertyDisplayer.java:729)
        at
org.openide.explorer.propertysheet.PropertyDisplayer$9.focusLost(PropertyDisplayer.java:633)
        at java.awt.AWTEventMulticaster.focusLost(AWTEventMulticaster.java:171)
        at java.awt.Component.processFocusEvent(Component.java:3642)
        at javax.swing.JComponent.processFocusEvent(JComponent.java:2058)
Comment 1 David Strupl 2001-02-15 14:05:59 UTC
>In my property editor, in setText, if the value is illegal,
> I show an alert dialog with the error.

How do you show the error dialog? (I assume by throwing
an exception - what kind of exception?)

> Now it comes up twice
> if the value is illegal.  I added a Thread.dumpStack() to my

I have added System.out to my test code and seems ok. Strange.

> code and found PropertyDisplayer applying the changes twice:
> once in actionPerformed and once in focusLost.  For a user,
> it's quite annoying to get the error twice, but in general
> applying the value twice (even
if it is legal) is a bad thing.

It is definitely a bad thing. It should not happen. I hope
we can find what is the difference that makes it behave
badly (and fix it).

Maybe it would be best if you send
me a PropertyEditor that demonstrates the bad behaviour.
Thanks a lot.
Comment 2 Rochelle Raccah 2001-02-16 05:29:38 UTC
Well, I may be doing it incorrectly - I'm not throwing an exception, I'm putting
up a NotifyDescriptor with the error and returning.  That's probably the wrong
way to do it, but even so, why is the method called twice?

My setAsText has this structure:
public void setAsText (String string)
{
	if (string == null) || (string.trim().length() == 0)) 
		setValue(null);
	else
	{
		Object lookupObject = <some method based on the string.trim()>;

		if (lookupObject != null)
			setValue(lookupObject);
		else
		{
			NotifyDescriptor desc = 
				new NotifyDescriptor.Message(<some message>, 
				NotifyDescriptor.ERROR_MESSAGE);

			TopManager.getDefault().notify(desc);
		}
	}
}
Comment 3 David Strupl 2001-02-16 10:45:56 UTC
If you change Notify to System.out.println it is called just once! Now I can see
what is the problem. Trying to find a solution.
Comment 4 David Strupl 2001-02-16 13:24:21 UTC
I have changed property displayer (just now - it will
be in the next build) to better notify
the user about an error. You can use this code now:

    public void setAsText (String text) throws IllegalArgumentException {
        if (<condition>) {
	    setValue(<new constructed value>);
        } else {
            throw new RuntimeException(<your localized message (visible for user)>);
            //or if you want not to show the message to the user
            //throw new IllegalArgumentException(<your localized message (going
to the log)>);
	}
    }

Please do not show the message yourself - in such case I can't figure
out how to disable the focus lost event (and not to call the code
twice).
Comment 5 Rochelle Raccah 2001-02-16 17:10:46 UTC
If I throw the RuntimeException, will you put it in an alert box?  Do either of
them put the focus back to the text (in the property sheet) afterwards?  I would
think the correct behaviour would be:

- User enters an invalid text string
- An error dialog is shown
- User hits okay
- Focus goes back to the propert sheet with the (bad?) text highlighted
- The user can change it and try again, or escape or something to discard
Comment 6 Jesse Glick 2001-02-16 17:48:10 UTC
Shouldn't it instead be that you can throw an IllegalArgumentException, and if
you attached a localized message using ErrorManager, it will display it? I think
this is the preferred way to handle this sort of thing for the future, correct
me if I am wrong...
Comment 7 David Strupl 2001-02-19 10:44:55 UTC
Jesse is right - RuntimeE is not nice. Let it be IllegalArgumentE (which is also
RuntimeE btw). I will change the code and let you know.
And Rochelle is also right that the user should be returned back to editing
after hitting Enter with incorrect value. The problem here is that the user can
switch to other property e.g. by a mouseclick in which case we do the same as
when hitting Enter and in this case it wouldn't make much sense to force the
user going back to the wrong value. Any ideas how to solve this? (To split the
behaviour for Enter vs. focus lost?)
Comment 8 David Strupl 2001-02-19 14:39:48 UTC
So the correct use in setText is:

IllegalArgumentException iae = new IllegalArgumentException();
TopManager.getDefault().getErrorManager().annotate(iae, <user info>);
throw iae;

If the annotation is ommited the user will not see the exception (it will go
only to log). In both cases use IllegalArgumentE.

If you think that the user should be returned back to the edited property after
the error dialog please create a new issue with type ENHANCEMENT. Thanks.
Comment 9 Rochelle Raccah 2001-02-20 01:05:02 UTC
I don't think this solution is acceptable from a user perspective.  With my old
solution (showing the NotifyDescriptor.ERROR_MESSAGE), the user saw a nice error
dialog showing them what the problem is.  This solution makes the issue look
like an unhandled exception.  In my book, the user should never see the
Exception/Show Details dialog unless the exception is unhandled.  It is not user
friendly.  Is it possible for you to wrap this in a
NotifyDescriptor.ERROR_MESSAGE with the user info or will that get us back to
the problem of showing it twice?
Comment 10 David Strupl 2001-02-20 09:23:17 UTC
Not showing the exception would not take us to the bad old behaviour.
I don't know: IMHO there should be a way to see the exception. I am reassigning
this issue to our UI engineer. Rosta: it is in PropertyDisplayer lines 731-754.
You can easily change the line 751 to something else but I am not sure that
it is the right thing to do.
Comment 11 David Strupl 2001-02-21 14:04:19 UTC
*** Issue 8413 has been marked as a duplicate of this issue. ***
Comment 12 Rochelle Raccah 2001-03-19 20:27:08 UTC
This is marked as an enhancement, but I think it is a defect.  I'm tempted to
vote for it on the bug parade, but can't if it's an enhancement.  Here's why I
think it's a bug:

- I have no way to show the user a friendly alert if the property value is
invalid.  I have 2 choices:
1) Put up my own (friendly) alert in which case it shows up twice
2) Throw an IllegalArgumentException in which case it shows up once in an
exception window which is scary for the user.
Comment 13 Jaroslav Tulach 2001-04-04 10:05:19 UTC
I have seen a reference to this bug on nbdev@.

If I understand right, the problem could be reformulated as "Ugly UI of
ErrorManager.annotate", am I right?

If so, we can enhance the look of errormanager's notification to add nicer UI,
not mention "Exception" in title but "Warning", etc.
Comment 14 Svata Dedic 2001-04-04 11:11:05 UTC
Yarda, please, let's discuss the correct (or intended) usage of ErrorManager on
nbdev@, I have some questions too, but I didn't got to formulate & send them
yet. Thanks.
Comment 15 Rochelle Raccah 2001-04-04 17:53:08 UTC
Yarda's interpretation is correct, but I agree this should be discussed further
on nbdev.  In fact, I think a similar discussion yesterday is what prompted all
these comments added to this bug.
Comment 16 Jaroslav Tulach 2001-04-19 13:23:30 UTC
Making dependent on the request for enhancement of ErrorManager.
Comment 17 Rochelle Raccah 2001-04-26 01:05:21 UTC
I'm now seeing this 4 times instead of 2!  The first time 2 copies of the alert
come up, then when I dismiss those, 2 more come up, 1 at a time.
Comment 18 Jan Chalupa 2001-05-06 08:15:08 UTC
Target milestone -> 3.3
Comment 19 Rostislav Levy 2001-06-05 15:55:18 UTC
Reasign
Comment 20 David Strupl 2001-08-02 14:11:58 UTC
I would like to close this one. The reasons are:
1. The code in property sheet is changed to use ErrorManager
exclusively and not doing any strange manipulation with exceptions
2. There is RFE 11558 dealing with "nice looking" user messages from
ErrorManager
3. I have checked with latest dev build and throwing an annotated
exception ends up correctly with user message and the old value being
left in the property

So I am marking it as fixed - if you think that the issue is different
from 11558 please reopen. Thanks for your time. 
Comment 21 Marian Mirilovic 2002-02-08 14:20:30 UTC
verified in [nb_3.3]
Comment 22 Quality Engineering 2003-07-01 16:10:54 UTC
Resolved for 3.4.x or earlier, no new info since then -> closing.