delete 'lock' file in FF profile

[MatzFan]

Exising profile model can now be used to instantiate FF profile

Fixes #11576

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[CLAassistant]

All committers have signed the CLA.

[diemol]

It seems several files have been added, while the change applies only to a few. Can you please check and submit only the change set that was intended?

[MatzFan]

@diemol sorry I should have explained. The last 2 files are the test and functionality change. All the new files are a Firefox (v119) profile I've added to common/profiles (a new dir) in a dir named 0fbxm2ve.default-release. This was copied from a fresh install on my system with one change for my test, in prefs.js; user_pref("browser.aboutConfig.showWarning", false); (the default is true). The profile is a fixture to test that the Selenium::WebDriver::Firefox::Profile class can be instantiated with an existing model. I've put it in common as I figured other language bindings may want to use the fixture.

I could try and figure out what is unnecessary for the test and trim the profile files, assuming FF doesn't need them all.

[diemol]

We are moving away from profiles and recommending to use preferences. We'll definitely not add those files that belong to a profile.

[titusfortner]

There are things that cannot be done with preferences, and I think there has been some miscommunication on what Firefox is doing and what they request users to do — mozilla/geckodriver#1656 (comment)

That said, there is no way for us to know if the next version of Firefox changed the profile structure slightly and what we have would break things or need to be updated. The potential maintenance costs are too high.

[MatzFan]

Understood, I guess there are 2 choices then:

1. Ditch the profile fixture and the test and just keep the tiny change to Selenium::WebDriver::Firefox::Profile to fix the bug
2. Create a profile on disk (from a download of FF?) and remove it afterwards all within the test => no overhead of a fixture that needs maintaining. At least this way functionality is tested and any breaking changes Mozilla make to FF profile will at least be picked up with a breaking test.

My own view is that using an existing profile is very useful, can work with this tiny change (one word) and therefore ought to be supported and tested. If there are no objections I'll edit the PR to do 2).

[MatzFan]

Gone for option 1) for now to fix #11576. Will do new PR with a test once I figure out how to create a FF profile directory in a test.

[MatzFan]

So this now fails on Windows with

 KeyError:
       key not found: "APPDATA"
     # ./rb/lib/selenium/webdriver/firefox/util.rb:30:in `fetch'
     # ./rb/lib/selenium/webdriver/firefox/util.rb:30:in `app_data_path'
     # ./rb/lib/selenium/webdriver/firefox/profiles_ini.rb:28:in `initialize'
     # ./rb/spec/integration/selenium/webdriver/firefox/profile_spec.rb:31:in `new'
     # ./rb/spec/integration/selenium/webdriver/firefox/profile_spec.rb:31:in `profile_model'
     # ./rb/spec/integration/selenium/webdriver/firefox/profile_spec.rb:28:in `block (2 levels) in <module:Firefox>'
     # ./rb/spec/integration/selenium/webdriver/firefox/profile_spec.rb:65:in `block (2 levels) in <module:Firefox>'

I can edit the test code to rescue this, but I'd guess this problem is CI not having the correct env vars. If that is right can someone add APPDATA to the win environment? Thx.

[titusfortner]

Weird, even this indicates it should be available: https://stackoverflow.com/questions/75669170/getting-the-appdata-roaming-folder-inside-windows-github-action

[titusfortner]

Maybe we try LOCALAPPDATA insteaqd?

[MatzFan]

Can't get test to pass in CI and can't spend any more time on this so have reverted to simple change to delete the 'lock' file to fix #11576

[titusfortner]

Sorry I haven't had time to look at the failures. Nothing looks obvious for why it isn't happy 😦

[MatzFan]

No worries @titusfortner it has passed once with my final commit so the failures are intermittent ones 😄