Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Description • 6 years ago

With Firefox 44 the pref "browser.tabs.animate" got removed, and different other animation prefs were moved into "toolkit.cosmeticAnimations.enabled":

https://dxr.mozilla.org/mozilla-central/rev/0ee6b755ab2ee6d2ab79b17cc97bd4e83424cbfc/browser/components/nsBrowserGlue.js#1969-1981

Those include:

Services.prefs.clearUserPref("browser.tabs.animate");
Services.prefs.clearUserPref("browser.fullscreen.animate");
Services.prefs.clearUserPref("alerts.disableSlidingEffect");

I think especially getting the fullscreen animation disabled would help a lot. Maybe that's the place where Selenium users have issues.

Note that Marionette already uses the new pref for a while.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 1 • 6 years ago

Well UI version doesn't actually map to Firefox version. So the actual change landed in Firefox 55 via bug 1352069.

Given that we only support Firefox 55 and greater we can make that change.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 2 • 6 years ago

Actually this preference is already set as recommended pref in marionette.js. I doubt that it requires a restart of Firefox, so we can actually get it removed from prefs.rs. I will have a look into it.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 3 • 6 years ago

I will change the summary of the bug so that it covers all preferences which are inappropriately set at both locations. We only have to set preferences in prefs.rs, when those aren't live-updated while Firefox is running.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 4 • 6 years ago

Basically I checked which prefs do not exist anymore. Those we can remove. Otherwise we should try to get as many prefs out of prefs.rs and move them into marionette.js to make geckodriver lesser Firefox version dependent, and that we don't have to carry around that many outdated prefs. Instead depending on the version of Firefox Marionette will set them. Note, that some were not present yet in marionette.js, and removal in prefs.rs has to wait until the appropriate version of Firefox is the minimal supported release for a geckodriver release.

What we definitely have to keep in prefs.rs are those preferences which need a restart of Firefox, or affect startup behavior.

Here some prefs which needed some investigation:

* "browser.EULA.override" isn't used anywhere except for tests. As it looks like it has been renamed a very long time ago to "browser.rights.override" (see bug 456439). Then with bug 819493 for Firefox 23 this moved from a notification bar to a snippet. Now with bug 1409054 it got removed for Firefox 62. But via bug 1463277 there is a discussion if a first run page should be shown or not. I feel this needs to be a separate bug for all test harnesses. I will remove it and observe the two other bugs for possible alternative implementations if wanted.

* "browser.offline" got removed for Firefox 7 in bug 663253. Since then the offline status is managed by the network itself and there is a pref called "network.manage-offline-status" which geckodriver already uses. So I will remove the old pref.

* "browser.reader.detectedFirstArticle" got removed in Firefox 55 via bug 1352501.

* "browser.usedOnWindows10.introURL" isn't used anywhere and got removed between Firefox 45ESR and 52ESR. It's safe to be removed.

* "browser.urlbar.userMadeSearchSuggestionsChoice" is no longer in use since Firefox 55. Here the UI upgrade path: https://dxr.mozilla.org/mozilla-central/rev/d8f180ab74921fd07a66d6868914a48e5f9ea797/browser/components/nsBrowserGlue.js#1993-2018. The new preference is "browser.urlbar.suggest.searches".

* "extensions.showMismatchUI" got removed for Firefox 61 via bug 1433574. It was only important for users which still had old-style extensions enabled, and upgraded from <57 to 60 (ESR). It's
not needed for geckodriver when we set the minimum supported version to 57.

* "javascript.options.showInConsole" - not sure about this one. I cannot find any usage of this preference even it still gets set with a default value for Firefox and Fennec. I will leave it as is.

* "network.http.bypass-cachelock-threshold" got removed in Firefox 55 via bug 1361435. It's safe to remove it.

* "network.sntp.pools" no idea about that one. There doens't seem to be any code using this pref, but mozprofile still sets it. Maybe lets leave for now.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 5 • 6 years ago

As best I would like to get this also uplifted to Firefox 61, so that we can remove remaining prefs in prefs.rs earlier.

	Andreas Tolfsen ❲:ato❳
	Comment 7 • 6 years ago
mozreview-review

Comment on attachment 8983349 [details]
Bug 1466658 - [geckodriver] Overhaul of user preferences in prefs.rs.

https://reviewboard.mozilla.org/r/249240/#review255432

::: commit-message-0ee6b:3
(Diff revision 1)
> +Lots of preferences aren't used anymore, or are safe to be set in
> +marionette.js to make them version specific to Firefox.

“or are safe to set at startup”?

::: testing/geckodriver/CHANGES.md:11
(Diff revision 1)
>  
>  Unreleased
>  ----------
>  
>  Note that with geckodriver 'next' the following versions are recommended:
> -- Firefox 56.0 (and greater)
> +- Firefox 57.0 (and greater)

You should probably also mention in the commit message that this
patch bumps the recommended version of Firefox to 57.

::: testing/geckodriver/CHANGES.md:11
(Diff revision 1)
>  
>  Unreleased
>  ----------
>  
>  Note that with geckodriver 'next' the following versions are recommended:
> -- Firefox 56.0 (and greater)
> +- Firefox 57.0 (and greater)

Also change the version number in testing/geckodriver/README.md.

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 8 • 6 years ago
mozreview-review-reply

Comment on attachment 8983349 [details]
Bug 1466658 - [geckodriver] Overhaul of user preferences in prefs.rs.

https://reviewboard.mozilla.org/r/249240/#review255432

> You should probably also mention in the commit message that this
> patch bumps the recommended version of Firefox to 57.

I also added an entry to Changes.md so we don't forget about it.

> Also change the version number in testing/geckodriver/README.md.

So many places.... done.

	Pulsebot
	Comment 10 • 6 years ago

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f8c98ae75bbf
[geckodriver] Overhaul of user preferences in prefs.rs. r=ato

	Arthur Iakab [arthur_iakab]
	Comment 11 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/f8c98ae75bbf

	Henrik Skupin [:whimboo][⌚️UTC+2] Assignee
	Comment 12 • 6 years ago

Please land this test-only patch on beta.

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 6 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/ebdb496585ab