_WD_GetBrowserPath() does not return the full path

[mlipok]

Bug report

Describe the bug

A clear and concise description of what the bug is.

How to reproduce

_WD_GetBrowserPath('firefox')

returns "FIREFOX.EXE" instead true/full path.

@="FIREFOX.EXE"
"Path"=""

Expected behavior

To be rethinking what to do.

Screenshots

Additional context

None

System under test

Please complete the following information.

• Browser FireFox
• Browser version 100.0 64Bit

https://www.mozilla.org/en-US/firefox/100.0/releasenotes/?utm_source=firefox-browser&utm_medium=firefox-desktop&utm_campaign=about-dialog

[Danp2]

For some reason your registry isn't correct. Here's what I get --

@="C:\\Program Files\\Mozilla Firefox\\firefox.exe"
"Path"="C:\\Program Files\\Mozilla Firefox"

Since the function is designed to retrieve the registry settings, I'm not sure how you would propose to "fix" something that isn't broken. 😃

Have you tested this with other browsers? If so, what was the result?

[mlipok]

I solve this with this code:

If $s_FireFox_Binary = Default Then $s_FireFox_Binary = _WD_GetBrowserPath('firefox')
If Not FileExists($s_FireFox_Binary) Then
	If FileExists(@LocalAppDataDir & "\Mozilla Firefox\firefox.exe") Then
		$s_FireFox_Binary = @LocalAppDataDir & "\Mozilla Firefox\firefox.exe"
	EndIf
EndIf

I think there is a need to check if file exist, and in case of not existen file returned by _WD_GetBrowserPath there should be $_WD_ERROR_FileIssue set as @error

[mlipok]

also we can add If FileExists(@LocalAppDataDir & "\Mozilla Firefox\firefox.exe") Then as an additionall check in case when registry entries doesnt point you to existen file.

[Danp2]

A few suggestions if you decide to expand the functionality of _WD_GetBrowserPath --

• Expand $_WD_SupportedBrowsers to hold these predefined locations
• Update the description in the header to include "predefined locations on disk"
• Check the error codes in the header for accuracy

[mlipok]

I notice that my Chome is located: C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

As I have Chrome 64bit version I start to wonder why this is not C:\Program Files\Google\Chrome\Application\chrome.exe

EDIT: the sam with Edge C:\Program Files (x86)\Microsoft\Edge\Application\msedge.exe

EDIT 2: My doubts are related to the fact that the catalog C:\Program Files (x86)\..... to the best of my knowledge, should be used as a location for 32-bit programs, not 64-bit programs.

[Danp2]

All of the above makes me second guess these potential changes to _WD_GetBrowserPath. The current implementation does the job it was intended to do, which is to retrieve the installation path from the registry. Looking for it on disk is complicated because there are so many places it could be.

[mlipok]

So we should not look everywhere.

I think there is a need to check if file exist, and in case of not existen file returned by _WD_GetBrowserPath there should be $_WD_ERROR_FileIssue set as @error

But do we should check if the path exist ?

[Danp2]

But should we check if the path exists?

I don't believe this is needed. If you do decide to add this, then be sure that it doesn't change the existing functionality (ie: you can set an error code, but the registry path should still be returned).

[mlipok]

I will add $_WD_ERROR_FileIssue but I agree and I will follow you recomendation:

then be sure that it doesn't change the existing functionality (ie: you can set an error code, but the registry path should still be returned).

I am temporarily holding back, however, in order to finish my current PR and stabilize the version I'm working on.

[mlipok]

I plan to take a look on this in next few days

[mlipok]

If you do decide to add this, then be sure that it doesn't change the existing functionality (ie: you can set an error code, but the registry path should still be returned).

I will add feaure which will set additionall ERRROR only in case when the registry entry is empty string, as in my screenshot and of course it will return this exact empty string. Is it fine for you ?

[Danp2]

That should be fine. However, it isn't clear if you intend to use @error or @extended to indicate this additional error. I would suggest using @extended.

[mlipok]

IMHO if the registry had an empty string as the path, then it should be marked as @error as the user expects that on success you will get a valid path.

[Danp2]

To clarify my earlier statement, I was trying to suggest that you use one of the existing error codes (ie: $_WD_ERROR_NotFound) along with a unique value in @extended to indicate that the registry key was empty.

[mlipok]

You can assign this to me.

[Danp2]

Closing this for now as the current implementation is performing as originally designed.