Add support for managed Firefox #78
Conversation
|
Hi -- thanks for the PR. @grandchild implemented Firefox support -- what do you think? it seems like the profile_dir and profile_name kwargs should be able to provide enough flexibility to get things working either way, but it does make sense that the most recently modified profile would be the one of greatest interest. |
|
Good morning @grandchild and @n8henrie, Just wanted to see if you'd had a chance to check over this change and make sure it makes sense. |
|
Hi, I only just got around to reviewing this. I agree with @n8henrie that one could achieve the same by specifying a profile dir explicitly. However I also agree that it may make sense to default to the most recent one. So the change itself looks good to me. However, because the Firefox install/profile stuff is pretty obscure, the commit message should have a lot more detail. How about the following: |
|
I'd also add that the most recently created install doesn't necessarily have to be the active one, it may be a dev/nightly version install. So this change could break things for others that don't have the assumed setup above (i.e. company managed). |
|
Hmm, that's a good point, thanks for your input. I had misunderstood this to mean the most recently modified profile, but the creation date I think is far less indicative. I can easily imagine users first creating a default profile, and only later discovering that they wanted to make a separate profile for work or what have you. I would plan on a "most significant" number bump for this given a breaking change in behavior, if merged, but I'm also wondering if keeping current behavior might not be best. @DukeSliver, have you been able to try the workaround I mentioned above? If so, did it work? |
Just to disambiguate, "install" != "profile". 99% of users will have only one install and perhaps multiple profiles. Basically, different installs are needed when you need different Firefox binaries (instead of just profiles) -- hence the dev usecase. I'd say it's pretty niche, and undisruptive either way. |
|
Thanks for clarification. I agree with the explanatory commit message. @grandchild I'll defer merging to you -- feel free! |
|
@n8henrie Yep passing kwargs works as well |
|
@grandchild any additional thoughts on this? |
|
@n8henrie nope, waiting for @DukeSliver to amend the commit message, as per above. @DukeSliver would you add the text above to the commit message? Then it's mergeable for me. |
|
Added it in |
|
Ah I meant editing the original commit message. Should I just do it and edit the branch? Git would show you as author and me as comitter then. /edit |
Default to last Firefox install if multiple When _not_ explicitly requesting a specific Firefox profile directory the default profile is chosen. However on top of profiles Firefox may have multiple "installs" (listed in the `installs.ini` file) each with their own default profile. Before, the default profile was the one from the first install. However, it makes more sense to pick the last one instead, assuming that that's the most recently created one, and thus the active one.
|
Ah I misunderstood, should be fixed now |
NB: Please make an issue prior to embarking on any significant effort towards
a pull request. This will serve as a unified place for discussion and hopefully
make sure that the change seems reasonable to merge once its implementation is
complete.
Description
Without the change, the wrong Firefox profile was being loaded and no cookies were coming through. My organization manages Firefox so we have multiple "default-release" profiles in the profiles.ini file. It seems with managed Firefox browsers, users will be stuck on the most recently created profile so I switched the profile selection behavior to grab the last profile instead of the first. In theory this shouldn't break anything since the most recently created profile should be the active one anyway.
Status
READY