OneDrive/SharePoint bug fix + Option Explicit + others...

[6DiegoDiego9]

Hi GCuser99!

Congratulations for your SeleniumVBA project, it looks very useful and promising!
I chose to use it as my primary Selenium library in my projects so I'll try to give my contribute to improve it.

I was going to submit a PR for the first set of modifications but since I renamed many files (adding a "Web" prefix for aggregation) and that was detected as new files by GitHub Desktop instead than changed files, I opted to write to you here instead.

I made the following changes:

1. fixed crash when the workbook is on a disk synched with OneDrive or SharePoint.
   See detailed info in my comments in the new ThisWorkbook_DiskPath function
2. moved the GetAbsolutePath (and the new ThisWorkbook_DiskPath) functions to a module, avoiding duplicated code in multiple classes
3. added "Option Explicit" and consequent required declarations to every module and class.
   This is useful for avoiding to leave unseen typos that may lead to unpredictable results.
   For example it allowed me to spot your typo "CheckComptibilityLevel = -1" at debug time, otherwise it would just silently create a new variable CheckComptibilityLevel assigning -1 to it instead than to the correct CheckCompatibilityLevel variable (!)
4. renamed the class names adding the Web prefix where not already there.
   The reason for this is to aggregate the classes related to this Selenium library in both the Project Explorer and IntelliSense, useful when the files are integrated (added) to an existent (other) project with other classes.
   I find that the first advantage of this library is to avoid dependencies, to be able to send or share a single xlsm/xlsb file to third persons, so for me the typical scenario is to integrate it to workbooks with other pre-existing modules and classes.

Here is the resulting seleniumvba_v1.4.2.zip

[GCuser99]

@6DiegoDiego9,

Great set of improvements - thanks! I will go ahead and commit your changes pretty much untouched, except for the following line in GetInstalledDriverVersion function of WebDriverManager class changing:

Dim vparts() As Variant

to:

Dim vparts As Variant

I was on the verge of adding Firefox browser support and reworking the WebDriverManager class, so if have any more suggestions please get them to me soon so that we can minimize mis-synchronization. I will commit your changes along with my planned changes within the next week or so...

Good work!

GCUser99

[6DiegoDiego9]

Hi!
Thank you very much for your fast answer!
I'll do my best to manually track my future changes (if any) until the repository will be aligned to my files, then hopefully I will use the PR feature.

About the declaration of vparts, I intentionally added the array specification "()" because Split returns an array and by principle I use the stricter declaration, although your wider declaration of course will work too because of the mutable nature of the Variant.
vparts() ensures that nothing that is not an array will be ever assigned to it. Plus, maybe it's faster than vparts because maybe there's one less mutation when Split assigns an array to it, although I'm still not sure whether this logic assumption is in fact true.
Anyway, perhaps the most convincing argument for us is that MS does the same in this official example: https://docs.microsoft.com/en-us/office/vba/language/reference/user-interface-help/split-function

About the Firefox browser support, I'm no more interested in it since it's used by a declining about 3% of users while Chrome and Edge now use the same Blink rendering engine accounting for about 70% users.
Furthermore, "F12" of Chrome and Edge and their extensions and compatible apps (eg Fiddler) are as far as I see no more behind those of Firefox like many years ago.
What are some reasons to still use Firefox?

It's more likely that I'll have to change WebDriverManager because it hangs sometimes when used with the automatic alignment of WebDriver, as I always do. I'll likely go into it and fix it soon.

I'll keep you updated :)

[GCuser99]

On the vparts declaration issue...

My system throws an error (Type mismatch) if I declare vparts as a variant array as you did - not sure why that happens and does seem to contradict the MS example you sited above. However, it does not throw an error if I declare as a string array, which is better coding practice anyway, so that is what I ended up doing.

I don't use FireFox myself, but I have already done most of the necessary work and have determined that adding it should not affect the Chrome/Edge functionality.

I'm curious if you have a need for Edge IE mode? I am thinking not to add that to production, as I have documented some significant problems with the driver.

[6DiegoDiego9]

vparts: oh I have the "Type mismatch" error too and I completely agree with your solution: String array is the best practice here.
I just thought it was like Array = ("one","two","three"), that can't be assigned directly to a String array.

Edge IE mode: I think I will not need it. Yes I saw that you found a flaky driver. Not worth the effort imho.

[GCuser99]

Hi @6DiegoDiego9,

I've been working on an alternative AddIn solution for implementing SeleniumVBA. I'm wondering if you can help me out with something regarding the OneDrive/SharePoint thisworkbook.Path issue that you identified above...

Do you find a similar issue with this piece of code?

Dim fso As New Scripting.FileSystemObject

'this is the line I'm concerned about - does it return local path or does it share the same problem as thisworkbook.Path?:
filePath = Application.vbe.ActiveVBProject.Filename()

folderPath = fso.GetParentFolderName(filePath)

Thanks in advance!

[6DiegoDiego9]

Here are the results:

?ThisWorkbook.Path
https://d.docs.live.net/e06a[...]/Documenti/AUTO

?ThisWorkbook.FullName
https://d.docs.live.net/e06a[...]/Documenti/AUTO/MASTER.xlsm

?Application.vbe.ActiveVBProject.Filename()
https://d.docs.live.net/e06a[...]/Documenti/AUTO/MASTER.xlsm

?oFso.GetParentFolderName(Application.vbe.ActiveVBProject.Filename())
https://d.docs.live.net/e06a[...]/Documenti/AUTO

and I opened the file from "C:\Users\[myUsername]\OneDrive\Documenti\AUTO"

[GCuser99]

@6DiegoDiego9, I'm considering the following refactor of GetAbsolutePath() to accommodate an alternative Add-In solution (coming soon). I would like to have to manage only one set of codes for both the current SeleniumVBA setup and the Add-in, hence I plan on making the changes below to the current code base prior to generating the alternative Add-in file. I have made an attempt not to break anything in the current code with these changes but given that I do not have a OneDrive setup to test on, I cannot be sure that I have covered all of the bases correctly.

The important changes:

• Adding the Optional referenceFolderPath input argument to GetAbsolutePath() (as in here)
• Generalizing your ThisWorkbook_Path() function to GetLocalOneDrivePath() for use on any OneDrive url-type path
• Added a few helper path functions for increased clarity and functionality

I thought I should throw this out there for your comments (if any) before I commit. Of course if we run into trouble after the commit, then we can address the problems as we go.

Cheers.

Option Explicit

Private Declare PtrSafe Function SetCurrentDirectory Lib "kernel32" Alias "SetCurrentDirectoryA" (ByVal lpPathName As String) As Long
Private Declare PtrSafe Function PathIsRelative Lib "shlwapi" Alias "PathIsRelativeA" (ByVal pszPath As String) As Long
Private Declare PtrSafe Function PathIsURL Lib "shlwapi" Alias "PathIsURLA" (ByVal pszPath As String) As Long

Public Function GetAbsolutePath(ByVal inputPath As String, Optional ByVal referenceFolderPath As String = "") As String
    Dim fso As New Scripting.FileSystemObject, savePath As String

    'make sure no rogue beginning or ending spaces
    inputPath = VBA.Trim(inputPath)

    If Not IsPathRelative(inputPath) Then 'its an absolute path - avoid the hell below and get out now
        'OneDrive/SharePoint user could have specified a path built with ThisWorkbook.Path? Just in case...
        If IsPathHTTPS(inputPath) Then inputPath = GetLocalOneDrivePath(inputPath)
        GetAbsolutePath = inputPath
    Else 'ok then convert relative path to absolute
        'make sure no unintended beginning or ending spaces
        referenceFolderPath = VBA.Trim(referenceFolderPath)
        
        If referenceFolderPath = "" Then referenceFolderPath = ThisWorkbook.Path
        
        'its possible that user specified a relative reference folder path - convert it to absolute relative to ThisWorkbook.Path
        If IsPathRelative(referenceFolderPath) Then referenceFolderPath = GetAbsolutePath(referenceFolderPath, ThisWorkbook.Path)

        'convert OneDrive path if needed
        If IsPathHTTPS(referenceFolderPath) Then referenceFolderPath = GetLocalOneDrivePath(referenceFolderPath)
        
        'check that reference path exists and notify user if not
        If Not fso.FolderExists(referenceFolderPath) Then
            MsgBox "Reference folder path does not exist." & vbCrLf & vbCrLf & referenceFolderPath & vbCrLf & vbCrLf & "Please specify a valid folder path.", , "SeleniumVBA"
            End 'execution
        End If
        
        'employ fso to make the conversion of relative path to absolute
        savePath = CurDir()
        SetCurrentDirectory referenceFolderPath 'VBA ChDrive/ChDir don't work with UNC paths, see https://stackoverflow.com/questions/57475738/
        GetAbsolutePath = fso.GetAbsolutePathName(inputPath)
        SetCurrentDirectory savePath
    End If
End Function

Public Function GetLocalOneDrivePath(ByVal strPath As String) As String
    '
    ' thanks to 6DiegoDiego9 for doing research on this
    '
    ' The reason for this function is that when the workbook is opened on a disk synched with OneDrive or SharePoint,
    ' (ThisWorkbook.FullName and) ThisWorkbook.Path returns the correspondent cloud URLs instead than the original path on disk.
    ' For example:
    ' "https://d.docs.live.net/e06a[etc...]/MyDocumentFolder/MyFolder"
    ' or "https://mycompany.sharepoint.com/personal/MyName_Company_com/MyDocumentFolder/mycompany/Apps/BlaBla"
    ' causing problems if that path is used with other functions, like ChDrive.
    '
    ' This function will return the original/real path on disk.

    If IsPathHTTPS(strPath) Then
        'Original script taken from https://stackoverflow.com/a/72736800/11738627  (credits to GWD and his sources)
        Const HKEY_CURRENT_USER = &H80000001
        Dim objReg As WbemScripting.SWbemObjectEx 'changed to early binding by GCUser99
        Dim regPath As String
        Dim subKeys() As Variant
        Dim subKey As Variant
        Dim strValue As String
        Dim strMountpoint As String
        Dim strSecPart As String

        Static pathSep As String
        If pathSep = "" Then pathSep = Application.PathSeparator
    
        Set objReg = GetObject("winmgmts:{impersonationLevel=impersonate}!\\.\root\default:StdRegProv")

        regPath = "Software\SyncEngines\Providers\OneDrive\"
        objReg.EnumKey HKEY_CURRENT_USER, regPath, subKeys
        
        If IsArrayInitialized(subKeys) Then 'found OneDrive in registry
            For Each subKey In subKeys
                objReg.getStringValue HKEY_CURRENT_USER, regPath & subKey, "UrlNamespace", strValue
                If InStr(strPath, strValue) > 0 Then
                    objReg.getStringValue HKEY_CURRENT_USER, regPath & subKey, "MountPoint", strMountpoint
                    strSecPart = Replace(Mid(strPath, Len(strValue)), "/", pathSep)
                    GetLocalOneDrivePath = strMountpoint & strSecPart
        
                    Do Until Dir(GetLocalOneDrivePath, vbDirectory) <> "" Or InStr(2, strSecPart, pathSep) = 0
                        strSecPart = Mid(strSecPart, InStr(2, strSecPart, pathSep))
                        GetLocalOneDrivePath = strMountpoint & strSecPart
                    Loop
                    Exit Function
                End If
            Next subKey
        End If
    End If
        
    GetLocalOneDrivePath = strPath 'unchanged
End Function

Private Function IsPathRelative(ByVal sPath As String) As Boolean
    'for some reason, PathIsRelative returns 1 for a properly formed url
    If PathIsRelative(sPath) = 1 And PathIsURL(sPath) = 0 Then
        IsPathRelative = True
    Else
        IsPathRelative = False
    End If
End Function

Private Function IsPathURL(ByVal sPath As String) As Boolean
    If PathIsURL(sPath) = 1 Then
        IsPathURL = True
    Else
        IsPathURL = False
    End If
End Function

Private Function IsPathHTTPS(ByVal sPath As String) As Boolean
    If VBA.Left$(sPath, 8) = "https://" Then
        IsPathHTTPS = True
    Else
        IsPathHTTPS = False
    End If
End Function

Private Function IsArrayInitialized(ByRef arry() As Variant) As Boolean
    If (Not arry) = -1 Then IsArrayInitialized = False Else IsArrayInitialized = True
End Function

[6DiegoDiego9]

I fastly read it and it seems fine.
About 10 tests on my OneDrive synched PC: all OK.
However, be aware that some bad inputPath passed to it generate invalid results instead than raising error. Example:

?GetAbsolutePath("\foobar","c:\temp")
\foobar

Minor issue: is there a particular reason why you prefer vbCrLf to vbNewLine? I always use vbNewLine supposing it's more Mac compatible.

Also, the following code:

    If PathIsURL(sPath) = 1 Then
        IsPathURL = True
    Else
        IsPathURL = False
    End If

could be reduced to the one-liner:
If PathIsURL(sPath) = 1 Then IsPathURL = True Else IsPathURL = False
or the even shorter:
IsPathURL = Iif(PathIsURL(sPath) = 1, True, False)

but if you chose the long version for more readability, it's ok too :)

[GCuser99]

Thanks for the response!

I'm pretty sure the vbNewLine=vbCrLf but I would not know anything about Mac compatibility, and without a Mac, I don't plan on tackling that hornet's nest! :-)

I'll give some thought to the bad input example that you provided. The PathIsRelative api function interprets "\foobar" as a non-relative path, and so no matter what the optional "referenceFolderPath" is (defaults to ThisWorkbook.Path), GetAbsolutePath will return inputPath after doing your OneDrive check. I'm not smart enough to know whether there is a case where the syntax "\path" points to a valid path name.

BTW as an obliquely tangential issue - GetAbsolutePath obviously does more than just check the input for relativity - it also takes care of the OneDrive issue. So I wonder if we should rename it to ProcessPath, NormalizePath, etc... Any thoughts on that?

I'll make this next commit soon, to get things ready for the alternative Add-in solution that I am planning.

[6DiegoDiego9]

vbCrLf vs vbNewLine: this is the page that convinced me to always use vbNewLine: https://docs.microsoft.com/en-us/office/vba/language/reference/user-interface-help/miscellaneous-constants

"\foobar": you're right, it could be a valid relative path, remembering the MS-DOS times where we could use it as argument of "cd" to jump to the "temp" directory in the root of the current drive. Since the "current drive" is still an used concept in VBA, it still makes sense. We should just be careful to use such a path with instructions that support it.

"GetAbsolutePath" renaming: I'm not perfectly comfortable with that name too but not because of the OneDrive issue, more because I didn't immediately understand what it does by reading only the name of the function and its arguments: "GetAbsolutePath inputPath, refPath".
I was thinking more to something like "PathRelativeToAbsolute relPath, basePath" but I'm not perfectly comfortable with that too in this case.

[GCuser99]

I'll make the vbNewLine change for you.

I'm leaning towards GetFullPath(inputPath, basePath) as in here?

Or GetFullLocalPath to account for OneDrive check?

[6DiegoDiego9]

Having seen that MS page, I'd vote for this mix: GetFullLocalPath(path, basePath)

[GCuser99]

In case this ever has to be revisited - thought this looks interesting: https://gist.github.com/guwidoe/038398b6be1b16c458365716a921814d

[6DiegoDiego9]

Tested on my non-synchronized OneDrive folder and it correctly returned an empty string :)

I think this is the solution that I read and referred in a citation in our code (https://stackoverflow.com/a/72736800/11738627 → linked updated solution). If I remember well, I just kept our alternative solution because it's much more compact and they said it likely works in 99% real cases.

Let's see how it will go ;)

[GCuser99]

Yeah, I'm not looking to change something that is not broken... just wanted to document that solution is there in case something does break in future... I like what we have now :-)