Bug 1804219 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

See https://phabricator.services.mozilla.com/D163891#inline-903429

I think the current comments around preference updates are lacking or misleading:
- [geckoinstance.py](https://searchfox.org/mozilla-central/rev/a44deb21744de6269329b05461c55488a147f95e/testing/marionette/client/marionette_driver/geckoinstance.py#7-10): no mention of the existence of the other files
- [prefs.rs](https://searchfox.org/mozilla-central/rev/a44deb21744de6269329b05461c55488a147f95e/testing/geckodriver/src/prefs.rs#7-18): only mentions marionette.js (which is now RecommendedPreferences.sys.mjs I assume)
- [RecommendedPreferences.sys.mjs](https://searchfox.org/mozilla-central/rev/2ad13433da20a0749e1e9a10ec0ab49b987c2c8e/remote/shared/RecommendedPreferences.sys.mjs#36-39): the most complete comment, but it suggests to either a preference "here" or both in prefs.rs + geckoinstance.py iff this preference needs to be set before restart

But since not all consumers use RemoteAgent's RecommendedPreferences (geckoinstance.py forces `remote.prefs.recommended` to false which disables RecommendedPreferences) we should call out explicitly in those comments that some preferences have to be duplicated in geckoinstance.py. Right now it seems the decision to add a pref in geckoinstance.py is only based on the fact that the preference can be applied live or not.

Maybe a decision chart should be like:
- should the preference be set before starting the Browser?
  - yes: set it in prefs.rs
  - no: set it in RecommendedPreferences.sys.mjs (maybe mention that we can have protocol specific preferences as well there)
- additionally, if the preference is relevant outside of CI/Automation scenarios (give examples of Marionette python client usage to help the read understand the context here) also add it in geckoinstance.py
See https://phabricator.services.mozilla.com/D163891#inline-903429

I think the current comments around preference updates are lacking or misleading:
- [geckoinstance.py](https://searchfox.org/mozilla-central/rev/a44deb21744de6269329b05461c55488a147f95e/testing/marionette/client/marionette_driver/geckoinstance.py#7-10): no mention of the existence of the other files
- [prefs.rs](https://searchfox.org/mozilla-central/rev/a44deb21744de6269329b05461c55488a147f95e/testing/geckodriver/src/prefs.rs#7-18): only mentions marionette.js (which is now RecommendedPreferences.sys.mjs I assume)
- [RecommendedPreferences.sys.mjs](https://searchfox.org/mozilla-central/rev/2ad13433da20a0749e1e9a10ec0ab49b987c2c8e/remote/shared/RecommendedPreferences.sys.mjs#36-39): the most complete comment, but it suggests to either a preference "here" or both in prefs.rs + geckoinstance.py iff this preference needs to be set before restart

But since not all consumers use RemoteAgent's RecommendedPreferences (geckoinstance.py forces `remote.prefs.recommended` to false which disables RecommendedPreferences) we should call out explicitly in those comments that some preferences have to be duplicated in geckoinstance.py. Right now it seems the decision to add a pref in geckoinstance.py is only based on the fact that the preference can be applied live or not.

Maybe a decision chart should be like:
- should the preference be set before starting the Browser?
  - yes: set it in prefs.rs
  - no: set it in RecommendedPreferences.sys.mjs (maybe mention that we can have protocol specific preferences as well there)
- additionally, if the preference is relevant outside of CI/Automation scenarios (give examples of Marionette python client usage to help the read understand the context here) also add it in geckoinstance.py

Also, we should have this comment in a single spot and link to it from all 3 files, so that the information doesn't get out of sync.
See https://phabricator.services.mozilla.com/D163891#inline-903429

I think the current comments around preference updates are lacking or misleading:
- [geckoinstance.py](https://searchfox.org/mozilla-central/rev/a44deb21744de6269329b05461c55488a147f95e/testing/marionette/client/marionette_driver/geckoinstance.py#7-10): no mention of the existence of the other files
- [prefs.rs](https://searchfox.org/mozilla-central/rev/a44deb21744de6269329b05461c55488a147f95e/testing/geckodriver/src/prefs.rs#7-18): only mentions marionette.js (which is now RecommendedPreferences.sys.mjs I assume)
- [RecommendedPreferences.sys.mjs](https://searchfox.org/mozilla-central/rev/2ad13433da20a0749e1e9a10ec0ab49b987c2c8e/remote/shared/RecommendedPreferences.sys.mjs#36-39): the most complete comment, but it suggests to either a preference "here" or both in prefs.rs + geckoinstance.py iff this preference needs to be set before restart

But since not all consumers use RemoteAgent's RecommendedPreferences (geckoinstance.py forces `remote.prefs.recommended` to false which disables RecommendedPreferences) we should call out explicitly in those comments that some preferences have to be duplicated in geckoinstance.py. Right now it seems the decision to add a pref in geckoinstance.py is only based on the fact that the preference can be applied live or not.

Maybe a decision chart should be like:
- should the preference be set before starting the Browser?
  - yes: set it in prefs.rs
  - no: set it in RecommendedPreferences.sys.mjs (maybe mention that we can have protocol specific preferences as well there)
- additionally, if the preference is relevant outside of CI/Automation scenarios (give examples of Marionette python client usage to help the reader understand the context here) also add it in geckoinstance.py

Also, we should have this comment in a single spot and link to it from all 3 files, so that the information doesn't get out of sync.

Back to Bug 1804219 Comment 0