Forum > Designer

SOLVED Impossible to close a modal window...

<< < (8/8)

MarkMLl:
No need to "redeem" yourself :-)

But we do collectively want to help, and since- if there's a real screw-up- we'll need to look at the project and possibly form files the easiest thing would be if you used the IDE's Project -> Publish Project facility to dump the whole lot.

We anticipate that it might not contain any custom libraries you're using, but it's very likely that we'll be able to see the problem if we cut out a whole lot of the DB-related stuff etc. and focus on the form interaction.

MarkMLl

pjtuloup:
Thanks ! I didn't know the Project->Publish function. I will use it in the future!

rvk:

--- Quote from: pjtuloup on July 10, 2024, 03:11:30 pm ---To redeem myself I will indeed publish the entire code and indicate what it is for, in case it helps someone with the same needs as me.
--- End quote ---
Great job (also for showing the code).

I do have a few small remarks. And one big one. I hope you don't mind...

1) Creation of the modalform:

First. I take it you moved FormIdentif from being autocreated (in the project options>Forms).
That's ok. But then you used the same FormIdentif variable for creating the modal form.
Because FormIdentif is limited to your OnShow procedure (and you create and free it there), I would remove the FormIdentif from the unit uIdentification and create FormIdentif as LOCAL variable for OnShow. That way you can't make any mistakes later on.

Second... you might want to use the try/finally construction for when there is a problem (to make sure the form is really freed). It just good programming that way.

Third... you don't need Close for FormIdentif because it's a modal form and when you return, it's already closed.

O, and because you are going to free the form itself, you can use Create(nil) instead of Create(Self). You (normally) only use Self if you want that 'Self' to handle the destruction.

So:

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFormMaitre.ShowModalForm;var  FormIdentif22: TFormIdentif; // I just added 22 to make sure you don't use the other one which should be removedbegin  FormIdentif22 := TFormIdentif.Create(nil);  try    FormIdentif22.ShowModal;    if FormIdentif22.IsCanceled Then      ShowMessage('Abandon: ' + FormIdentif22.ReturnValue)    else      ShowMessage('Poursuite: ' + FormIdentif22.ReturnValue);  finally    FormIdentif22.Free;  end;end;
2) Using ModalResult for easy result of the modal form

Instead of IsCanceled you can use ModalResult. If you use the X button at the top, the ShowModal will always return mrCancel.
For the continue button you can set ModalResult := mrOk and for the cancel button you do ModalResult := mrCancel.
DON'T USE Self.Close; Just set ModalResult correctly. It will close the modal form automatically and return the given value as function result.

In the ShowModalForm you can change the IsCancelled check into (so you can remove the IsCancelled variable entirely):


--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---procedure TFormMaitre.ShowModalForm;var  FormIdentif22: TFormIdentif; // I just added 22 to make sure you don't use the other one which should be removed  Result: Integer;begin  FormIdentif22 := TFormIdentif.Create(nil);  try    Result := FormIdentif22.ShowModal;    if Result = mrOk then       ShowMessage('Poursuite: Accès autorisĂ©!') // also maybe save the user info??    else      ShowMessage('Abandon: Sortie de l''application!');  finally    FormIdentif22.Free;  end;end;
Above will make the login form a bit more streamlined and work according to correct ShowModal principles.

3) Now one which can be a very BIG problem... !!!


--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---ReqUserHabil := 'SELECT B.Iduser, B.Nom AS NOM ,B.Prenom AS PRENOM, B.Pwd AS PWD, B.IdDiv, C.IdProfil AS IDPROFIL, C.LibProfil AS PROFIL, E.LibDiv AS DIVISION FROM gusers AS B Left Join ghabil AS A on B.IdUser=A.IdUser Left Join gprofils AS C on C.IdProfil=A.IdProfil Left Join gdiv AS E on E.IdDiv=A.IdDiv WHERE B.Nom=';ReqUserHabil += #39+SaisUser.Text+#39; // <---- PROBLEMReqUserHabil += ';';
If you are going to use your program only internally, with people you trust, you can get away with this (although even than it's not advised).
But if you want to make it secure (why else have a login), you really need to take care of that SQL injection vulnerability.

ANYONE can mess up your database (delete records, delete the entire database, gain access, etc, etc) when you use SaisUser.Text like that and just append it to your SELECT statement.

See https://www.w3schools.com/sql/sql_injection.asp

So, even if you don't do anything with the first 2 things... that last one is definitely something you really need to look at and understand...

MarkMLl:
RVK got in first... complete with obligatory xkcd :-)

OP: Does that fix things?

MarkMLl

pjtuloup:

--- Quote from: rvk on July 10, 2024, 06:44:25 pm ---
3) Now one which can be a very BIG problem... !!!


--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---ReqUserHabil := 'SELECT B.Iduser, B.Nom AS NOM ,B.Prenom AS PRENOM, B.Pwd AS PWD, B.IdDiv, C.IdProfil AS IDPROFIL, C.LibProfil AS PROFIL, E.LibDiv AS DIVISION FROM gusers AS B Left Join ghabil AS A on B.IdUser=A.IdUser Left Join gprofils AS C on C.IdProfil=A.IdProfil Left Join gdiv AS E on E.IdDiv=A.IdDiv WHERE B.Nom=';ReqUserHabil += #39+SaisUser.Text+#39; // <---- PROBLEMReqUserHabil += ';';
If you are going to use your program only internally, with people you trust, you can get away with this (although even than it's not advised).
But if you want to make it secure (why else have a login), you really need to take care of that SQL injection vulnerability.

ANYONE can mess up your database (delete records, delete the entire database, gain access, etc, etc) when you use SaisUser.Text like that and just append it to your SELECT statement.

See https://www.w3schools.com/sql/sql_injection.asp

So, even if you don't do anything with the first 2 things... that last one is definitely something you really need to look at and understand...

--- End quote ---

Thanks rvk, I will study that.

Navigation

[0] Message Index

[*] Previous page

Go to full version