Page 1 of 1
bug in StdCoder.Select
Posted: Wed Jun 29, 2016 11:19 pm
by Josef Templ
StdCoder.Select is a Notifier for the selection box in the Tools->Decode form.
It contains a Dialog.Update(par) call, which confuses the selection box.
The box shows more or less random behavior when selecting/deselecting multiple lines.
The same error exists in BB1.6.
Code: Select all
PROCEDURE Select*(op, from, to: INTEGER);
VAR p: FileList; i: INTEGER;
BEGIN
IF (op = Dialog.included) OR (op = Dialog.excluded) OR (op = Dialog.set) THEN
IF NofSelections(par.list) = 1 THEN
i := 0; p := par.files;
WHILE ~par.list.In(i) DO INC(i); p := p.next END;
par.storeAs := p.name
ELSE par.storeAs := ""
END;
Dialog.Update(par) <=== this call needs to be removed
END
END Select;
- Josef
Re: bug in StdCoder.Select
Posted: Sat Jul 02, 2016 6:05 am
by Josef Templ
please comment
Re: bug in StdCoder.Select
Posted: Sat Jul 02, 2016 8:38 am
by luowy
I've done a test:
1,select 2 or more valid file names in a textview
2,Tools-->Encode File List
3,open the "Decode" Dialog by Inserting a command views at begining of the new document
4,test it !
yes, your fixup worked well.
the Update operation must has been done elsewhere,though I am not clear;
luowy
Re: bug in StdCoder.Select
Posted: Sat Jul 02, 2016 9:24 am
by Josef Templ
I also don't understand the details and I am not very happy with this lack of knowledge.
Is it generally forbidden to call Dialog.Update for a control from its own notifier
because of the potential danger of an endless recursion?
Can Dialog.Update be made to work even for this situation?
At what time is an update of which control possible without the need to call Dialog.Update?
All I know is that my fix looks good in tests.
May be the experts from Oberoncore can provide some comments.
- Josef
Re: bug in StdCoder.Select
Posted: Sat Jul 02, 2016 1:44 pm
by luowy
I check it again,found :
worked well as the
it's weird!
Re: bug in StdCoder.Select
Posted: Sun Jul 03, 2016 5:11 pm
by Josef Templ
Calling Dialog.UpdateString(par.storeAs) explicitly is an improved work around
because it avoids the dependency on the automatic update of storeAs that is not defined anywhere,
or is it?
The reason why it works is simple:
The Dialog.Update(par) updates ALL interactors contained in the address range covered by par.
This includes storeAs but also list, which is the Dialog.Selection interactor that shows the problem.
By calling Dialog.UpdateString(par.storeAs) the update is narrowed to storeAs only.
With respect to list, it is the same as not calling Dialog.Update at all.
There is a more fundamental flaw in the notifier logic of a selection box.
In a selection box that already has a selection, a Mouse-Left-Click on an unselected line
means a sequence of two operations:
1. deselect (exclude) what is currently selected
2. select (include) the line clicked at
There are multiple notifier calls in this case!
Actually, there is a separate notifier call for every single line that is part of an edit operation.
Whenever there are multiple notifier calls there is a problem with calling
Dialog.Update from the notifier. What happens is that the editing operation is
terminated with the state at the first notifier call. This explains the observed random-like behavior.
- Josef
Re: bug in StdCoder.Select
Posted: Mon Jul 04, 2016 12:31 pm
by Josef Templ
I found the relevant and buggy code in
HostCFrames.HandleCommand/Update
and I also found THE solution (after extensively trying dead ends).
Any Update of a selection box while it is performing a sequence of notifier calls to
sync with an editing operation must be delayed, i.e. there must be an Update after the
editing operation has been finished. This also improves the runtime behavior from quadratic to linear!
Two boolean flags need to be introduced in the SelectionBox:
Code: Select all
SelectionBox = POINTER TO RECORD (StdCFrames.SelectionBox)
i: Info;
num: INTEGER;
delayUpdate, needsUpdate: BOOLEAN <== new code required
END;
Update must be delayed:
Code: Select all
PROCEDURE (f: SelectionBox) Update;
VAR res, i, j, a, b: INTEGER; sel: BOOLEAN;
BEGIN
IF f.delayUpdate THEN f.needsUpdate := TRUE; RETURN END; <== new code required
...
The notifier loop must be protected from Update in HandleCommand for SelectionBox:
Code: Select all
c.delayUpdate := TRUE; c.needsUpdate := FALSE; <== new code required
i := 0;
WHILE i < c.num DO
j := i;
res := WinApi.SendMessageW(c.i.ctrl, WinApi.LB_GETSEL, j, 0);
IF c.sorted THEN j := WinApi.SendMessageW(c.i.ctrl, WinApi.LB_GETITEMDATA, j, 0) END;
c.Get(c, j, b);
IF (res # 0) & ~b THEN c.Incl(c, j, j)
ELSIF (res = 0) & b THEN c.Excl(c, j, j)
END;
INC(i)
END;
c.delayUpdate := FALSE; <== new code required
IF c.needsUpdate THEN c.Update END <== new code required
This change is not a workaround as my previous proposal was and also luowy's improvement is.
It is a true fix of the problem that works for any selection box and it does not require the special
rule that a selection box notifier must not call Dialog.Update for its own interactor.
With the solution found I think it is time to file an issue.
- Josef
Re: bug in StdCoder.Select
Posted: Tue Jul 05, 2016 9:42 am
by luowy
Josef Templ wrote:Two boolean flags need to be introduced in the SelectionBox:
How about one boolean flag?
Code: Select all
SelectionBox = POINTER TO RECORD (StdCFrames.SelectionBox)
i: Info;
num: INTEGER;
(*delayUpdate, needsUpdate: BOOLEAN*)
updating:BOOLEAN;
END;
Code: Select all
PROCEDURE (f: SelectionBox) Update;
VAR res, i, j, a, b: INTEGER; sel: BOOLEAN;
BEGIN
(*IF f.delayUpdate THEN f.needsUpdate := TRUE; RETURN END;*)
IF f.updating THEN RETURN END;
Code: Select all
PROCEDURE HandleCommand (wnd: WinApi.HANDLE; wParam, lParam: INTEGER);
(*c.delayUpdate := TRUE; c.needsUpdate := FALSE;*)
c.updating:=TRUE;
i := 0;
WHILE i < c.num DO
j := i;
res := WinApi.SendMessageW(c.i.ctrl, WinApi.LB_GETSEL, j, 0);
IF c.sorted THEN j := WinApi.SendMessageW(c.i.ctrl, WinApi.LB_GETITEMDATA, j, 0) END;
c.Get(c, j, b);
IF (res # 0) & ~b THEN c.Incl(c, j, j)
ELSIF (res = 0) & b THEN c.Excl(c, j, j)
END;
INC(i)
END;
(*c.delayUpdate := FALSE;*)
(*IF c.needsUpdate THEN c.Update END*)
c.updating:=FALSE;
c.Update();
END
it can worked same as two flag as far as I tested;
The HostCFrames.action do the checked update all the time,except the notifier procedure.
maybe it is the reason which I not understand before: worked after comment out Dialog.Update .
Re: bug in StdCoder.Select
Posted: Thu Jul 07, 2016 12:12 pm
by Josef Templ
I also considered the version with a single flag but rejected it.
The reason is that I don't want to do ANYTHING automatically, i.e. without calling Dialog.Update explicitly.
This way it is guaranteed that everything behaves exactly as before when Dialog.Update was not called explicitly
(and therefore Update was not called and this is the same now).
The flag doesn't hurt. Actually it doesn't even require extra memory because it fills a gap.
- Josef