issue-#181 fixing error handling of HostDialog.OpenExternal

Re: issue-#181 fixing error handling of HostDialog.OpenExter

Postby Josef Templ » Thu Nov 09, 2017 7:22 am

Zinn wrote:One differences is that windows need absolute path name for OpenExternal and does not open a file with relative path name (file not found).
This I already reported above. It doesn’t matter if you use ‘/’ or ‘\’ the behaviour is the same.

- Helmut


Under my Windows it is possible to use relative path names but I have to use \.
This does make a difference.

If relative path names don't work for you, they may be relative to a different base directory.

Try for example:
"Dialog.OpenExternal('Empty.odc')"
"Dialog.OpenExternal('.\Empty.odc')"
"Dialog.OpenExternal('./Empty.odc')"

- Josef
User avatar
Josef Templ
 
Posts: 2038
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#181 fixing error handling of HostDialog.OpenExter

Postby Zinn » Thu Nov 09, 2017 9:38 am

An odc file is not the normal use for OpenExternal.
Copy a pdf file into the BlackBox directory and repeat your tests with the pdf file.
- Helmut
Zinn
 
Posts: 472
Joined: Tue Mar 25, 2014 5:56 pm
Location: Frankfurt am Main

Re: issue-#181 fixing error handling of HostDialog.OpenExter

Postby Josef Templ » Thu Nov 09, 2017 10:08 am

Zinn wrote:An odc file is not the normal use for OpenExternal.
Copy a pdf file into the BlackBox directory and repeat your tests with the pdf file.
- Helmut


It is exactly the same with a .pdf or with a .txt file.
I have tested all of them under both Vista and 10.

The question IMHO is what is your current directory?
A relative path is resolved against the current directory and it is not always clear what
the current directory is. It is possible that WinApi.GetFullPathNameW resolves it differently.
In any case please repeat the test with \ and with a simple setup where you know what the current directory is.
Or execute Dialog.RunExternal('cmd') to see what the current directory is (the prompt).

- Josef
User avatar
Josef Templ
 
Posts: 2038
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#181 fixing error handling of HostDialog.OpenExter

Postby Robert » Thu Nov 09, 2017 10:26 am

Josef Templ wrote:A relative path is resolved against the current directory and it is not always clear what
the current directory is. It is possible that WinApi.GetFullPathNameW resolves it differently.

Another confusing area!. What is the current path, & what does GetFullPathNameW fetch?

I use "Files.dir.This ('')(HostFiles.Locator).path$" to get the Client path, and "WinApi.GetModuleFileNameW" to get the Server path, but maybe there are better ways.
User avatar
Robert
 
Posts: 1023
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: issue-#181 fixing error handling of HostDialog.OpenExter

Postby Josef Templ » Thu Nov 09, 2017 2:05 pm

I would expect the current directory to be the one returned by WinApi.GetCurrentDirectoryW().
This is used in HostDialog for initializing the 'file open' dialog.

It may differ from the blackbox.exe directory (=GetModuleFileNameW) and the /USE directory.
You can set it for example in a link used for starting BlackBox.

- Josef
User avatar
Josef Templ
 
Posts: 2038
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#181 fixing error handling of HostDialog.OpenExter

Postby Josef Templ » Fri Nov 10, 2017 9:09 pm

I have done additional tests with GetFullPathNameW but I have not found any differences in the result.
Using a relative path (with \)opens the same file as using the corresponding absolute path.
The only difference is that GetFullPathNameW also converts all / to \.

IMHO, if you use Windows to open a relative path, you should adhere to the Windows naming conventions,
which means you should use \.
In addition, and even more importantly, by retrying the absolute path in OpenExternal but not in RunExternal, we get an ugly
difference in the behavior of both. Note that expanding to absolute paths in RunExternal is not easily possible because many commands have arguments,
not just a plain file name.

I would therefore prefer to stay with the simple form for now. At least until there is a reproducible problem
with the current strategy. (If you want to experiment with absolute file name expansion,
add the following lines before the line with "IF res = 0 THEN" into HostDialog.OpenExternal:
Code: Select all
      IF (res = 0) & (error = WinApi.ERROR_FILE_NOT_FOUND) THEN (* retry with absolute path *)
         res2 := WinApi.GetFullPathNameW(fileName, LEN(fileName2), fileName2, NIL); (* also converts / to \ *)
         IF fileName2 # fileName THEN
            sxinfo.lpFile := fileName2;
            res := WinApi.ShellExecuteExW(sxinfo);
            error := WinApi.GetLastError();
            IF res = 1 THEN RETURN 0
            ELSIF (res = 0) & (error = WinApi.ERROR_FILE_NOT_FOUND) THEN
               fileName := fileName2 (* use absolute path in error msg *)
            END
         END
      END;
)

The latest version has been updated significantly with respect to error handling.
I found out (by accident) that it is possible to get textual error messages for ALL Windows error codes returned by GetLastError().
This means that we don't need to extend the Host Strings resources to include a subset of error messages.
In order to use the error reporting, however, it was necessary to switch from using ShellExecute to ShellExecuteEx,
which does basically the same but delivers standard Windows error codes.

As it turned out, there was also a trivial bug in the definition of WinApi.FormatMessageW, which I fixed.

See diffs at https://redmine.blackboxframework.org/projects/blackbox/repository/diff?utf8=%E2%9C%93&rev=a48539efd6513aa41eeb91155f5a6505398ecd96&rev_to=22608024f21a9f8f22d42208399b8263cce82514.

For testing use http://blackboxframework.org/unstable/issue-%23181/blackbox-1.7.1-rc1.991.zip.

- Josef
User avatar
Josef Templ
 
Posts: 2038
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#181 fixing error handling of HostDialog.OpenExter

Postby Josef Templ » Sat Nov 11, 2017 10:12 am

Small refinement for working around another bug in wine.
wine's ShellExecuteEx does not always set the last error number correctly but returns 0 instead, indicating "success".
So I had to check that and fill in a 'most probable' error number.

Plus some minor code cleanups.

See diffs at https://redmine.blackboxframework.org/projects/blackbox/repository/diff?utf8=%E2%9C%93&rev=85506decc19da76df27e31b6008fe3ef72fa5cb9&rev_to=a48539efd6513aa41eeb91155f5a6505398ecd96.

The latest version for testing is http://blackboxframework.org/unstable/issue-%23181/blackbox-1.7.1-rc1.992.zip.

- Josef
User avatar
Josef Templ
 
Posts: 2038
Joined: Tue Sep 17, 2013 6:50 am


Re: issue-#181 fixing error handling of HostDialog.OpenExter

Postby Zinn » Tue Nov 14, 2017 4:10 pm

I have a small improvement:
Please move the new procedures FormatErrorMsg and ShowErrorDialog in front of procedure Start.
Then we can add error report to procedure Start too.
- Helmut
Zinn
 
Posts: 472
Joined: Tue Mar 25, 2014 5:56 pm
Location: Frankfurt am Main

Re: issue-#181 fixing error handling of HostDialog.OpenExter

Postby Josef Templ » Tue Nov 14, 2017 10:32 pm

Zinn wrote:I have a small improvement:
Please move the new procedures FormatErrorMsg and ShowErrorDialog in front of procedure Start.
Then we can add error report to procedure Start too.
- Helmut


changed.

See diffs at https://redmine.blackboxframework.org/projects/blackbox/repository/diff?utf8=%E2%9C%93&rev=2590c266e886582143bde3dad6a7e156a99fb7c6&rev_to=85506decc19da76df27e31b6008fe3ef72fa5cb9.

- Josef
User avatar
Josef Templ
 
Posts: 2038
Joined: Tue Sep 17, 2013 6:50 am

Previous

Return to Resolved (Bugs)

Who is online

Users browsing this forum: No registered users and 2 guests

cron