Version: HEAD (using KDE KDE 3.5.1) OS: Linux STEPS TO PREODUCE: Start Kst Create a curve Select Data... New Curve... In the "X axis vector" field type in the name of a vector that does not exist Hit OK RESULTS: Kst crashes EXPECTED RESULTS: Kst should ensure that the vectors exist before continuing
That's a kdFatal(). It used to be an impossible case but the UI change made it no longer impossible. -- George Staikos KDE Developer http://www.kde.org/ Staikos Computing Services Inc. http://www.staikos.net/
This happens for several dialogs that inherit KstDataDialog: kst/src/libkstapp/ksthsdialog_i.cpp: kstdFatal() << "Bug in kst: the Vector field (Hs) refers to " kst/src/libkstapp/ksthsdialog_i.cpp: kstdFatal() << "Bug in kst: the Vector field in hsdialog refers to " kst/src/libkstapp/kstpsddialog_i.cpp: kstdFatal() << "Bug in kst: the vector field (spectrum) refers to " kst/src/libkstapp/ksteqdialog_i.cpp: kstdFatal() << "Bug in kst: the Vector field (Eq) " kst/src/libkstapp/ksteqdialog_i.cpp: kstdFatal() << "Bug in kst: the Vector field (Eq) " kst/src/libkstapp/kstcsddialog_i.cpp: kstdFatal() << "Bug in kst: the vector field in spectrogram dialog refers to " kst/src/libkstapp/kstcurvedialog_i.cpp: kstdWarning() << "Bug in kst: the XVector field refers to " With the string freeze, I can't stop it from reporting 'Bug in kst' which kind of rules out a message box instead of throwing a fatal. I can make it so that it doesn't crash though, but that will mean dialogs fail to close when the 'ok' button is pressed and the user will receive no graphical message... just a warning to the console. I think this should wait for 1.5 and a proper fix.
The implications for leaving this as is are users experiencing crashes. What are the implications for introducing a new string?
We'd be introducing several new strings as this affects multiple dialogs and we are in a string freeze. I could also just add a blank error KMessageBox, but not sure that helps the user much.
We cannot introduce new strings now. A new string means no 1.4.0 release until May. -- George Staikos KDE Developer http://www.kde.org/ Staikos Computing Services Inc. http://www.staikos.net/
I'd still be interested to know what the implications are of introducing a new string at this point. Do we make all translated version unusable, or do they just get a blank string instead of the newly added string? A blank message box is better than a crash. I would suggest that if there are good reasons not to introduce new strings we simply set the focus back to the vector field and keep the dialog open. Not ideal but better than crashing.
Introducing a new string means the translation team ships with bugs. It's not fair to them to do this. We introduce a bug on their side in order to fix a bug on our side. We committed to a hard freeze and we have to stick with it or cancel our release. -- George Staikos KDE Developer http://www.kde.org/ Staikos Computing Services Inc. http://www.staikos.net/
Why are we talking about string changes? this is a bug in the KstComboBox.... it is suppose to act as a non-editable combo, and so it should never give you an invalid field name. If you do the reported action (enter an invalid X axis tag name) and then do something that removed focus from the combo (like hit tab) rather than hitting OK straight away, then the field reverts to whatever it was before you tried to change it. This is acceptable behavior. However, changing the field, then hitting apply doesn't remove focus, so this check never happens. The proper change is make sure that selectedVector() can never return a non-existant vector. This should be very easy to fix, and should be fixed for 1.4.0. Of course this leaves a UI bug: there is no visual indication that at any given moment, the combo box is in an invalid state. A visual queue which made clear whether what you have typed in is even possible would be good... (a mini icon? font change? colour change?) This is a 1.5 or 2.0 fix.
Right. Why was this changed to editable if it is not supposed to be? On 3-Apr-07, at 8:51 PM, Barth Netterfield wrote: > Why are we talking about string changes? this is a bug in the > KstComboBox.... > it is suppose to act as a non-editable combo, and so it should > never give you > an invalid field name. > > If you do the reported action (enter an invalid X axis tag name) > and then do > something that removed focus from the combo (like hit tab) rather than > hitting OK straight away, then the field reverts to whatever it was > before > you tried to change it. This is acceptable behavior. However, > changing the > field, then hitting apply doesn't remove focus, so this check never > happens. > > The proper change is make sure that selectedVector() can never > return a > non-existant vector. This should be very easy to fix, and should > be fixed > for 1.4.0. > > Of course this leaves a UI bug: there is no visual indication that > at any > given moment, the combo box is in an invalid state. A visual queue > which > made clear whether what you have typed in is even possible would be > good... > (a mini icon? font change? colour change?) This is a 1.5 or 2.0 > fix. > _______________________________________________ > Kst mailing list > Kst@kde.org > https://mail.kde.org/mailman/listinfo/kst -- George Staikos KDE Developer http://www.kde.org/ Staikos Computing Services Inc. http://www.staikos.net/
Doh! Of course. The combo wasn't changed to editable, it just appears that way because it is now searchable. I'll implement Barth's idea.
Created attachment 20176 [details] fix for crash Implement Barth's fix.
Looks good to me.
SVN commit 650551 by treat: * Don't allow non-existent values BUG:143803 M +5 -4 matrixselector.ui.h M +5 -1 scalarselector.ui.h M +5 -1 stringselector.ui.h M +5 -4 vectorselector.ui.h --- trunk/extragear/graphics/kst/src/widgets/matrixselector.ui.h #650550:650551 @@ -15,10 +15,11 @@ QString MatrixSelector::selectedMatrix() { - if (_provideNoneMatrix && _matrix->currentItem() == 0) { - return QString::null; - } - return _matrix->currentText(); + KstMatrixPtr ptr = *KST::matrixList.findTag(_matrix->currentText()); + if (!ptr || (_provideNoneMatrix && _matrix->currentItem() == 0)) + return QString::null; + else + return _matrix->currentText(); } --- trunk/extragear/graphics/kst/src/widgets/scalarselector.ui.h #650550:650551 @@ -192,7 +192,11 @@ QString ScalarSelector::selectedScalar() { - return _scalar->currentText(); + KstScalarPtr ptr = *KST::scalarList.findTag(_scalar->currentText()); + if (ptr) + return _scalar->currentText(); + else + return QString::null; } void ScalarSelector::allowDirectEntry( bool allowed ) --- trunk/extragear/graphics/kst/src/widgets/stringselector.ui.h #650550:650551 @@ -172,7 +172,11 @@ QString StringSelector::selectedString() { - return _string->currentText(); + KstStringPtr ptr = *KST::stringList.findTag(_string->currentText()); + if (ptr) + return _string->currentText(); + else + return QString::null; } void StringSelector::allowDirectEntry( bool allowed ) --- trunk/extragear/graphics/kst/src/widgets/vectorselector.ui.h #650550:650551 @@ -26,10 +26,11 @@ QString VectorSelector::selectedVector() { - if (_provideNoneVector && _vector->currentItem() == 0) { - return QString::null; - } - return _vector->currentText(); + KstVectorPtr ptr = *KST::vectorList.findTag(_vector->currentText()); + if (!ptr || (_provideNoneVector && _vector->currentItem() == 0)) + return QString::null; + else + return _vector->currentText(); }