1710356 - Don't hard-code product name as Firefox in "Browser.getVersion"

Status: RESOLVED FIXED

(Remote Protocol :: Agent, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

In the following line Firefox is hard-coded as product name:

https://searchfox.org/mozilla-central/rev/716d7cd80b3dcd81c005ad13b51d3e6a85bd7e73/remote/cdp/domains/parent/Browser.jsm#29

Instead Services.appinfo.name has to be used.

Comment 1

[Subramanian]

Can i work on it?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Yes, you can, and thanks for your interest.

Please see the user story section above in how to get started. If there are problems please let us know. Otherwise the bug will be automatically assigned to you once a patch has been uploaded.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Subramanian, did you face any problems in getting started? If that's the case please let me know how I can help. Thanks.

Comment 4

[Subramanian]

Sorry for the delay. The link seems to be an indexed search URI. Can I get the link to the actual file in the repo? Also though this is just as a replace change and you already mentioned the value, I would like to know where it is getting used.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So searchfox.org actually shows the path to the file, which in this case is remote/cdp/domains/parent/Browser.jsm. When you have cloned the unified repository as mentioned in the docs, you will find the file at this location.

Right now there are two places where it is getting used that are just integration tests:
https://searchfox.org/mozilla-central/search?q=Browser.getVersion&path=browser_

While these would still pass for Firefox we should also update these to use Services.appinfo.name as reference value for the product name.

I hope that answers all your questions. If not please let me know.

Comment 6

[Subramanian]

Pardon my ignorance Henik. As I see searchfox is a source indexing tool to make quick searches. Can I get the actual repo link (Like github/gitlab) to clone .git file or create a new branch and make this change quickly from web-ui itself and raise a Merge request? There is no actual url to mozilla central in the attached help doc.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Our docs refer to https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/unifiedrepo.html.

Comment 8

[manisai.valluri]

Is anyone working on this? Can I take this up ?

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hello Manisai. It's good to hear your interest. Given that we haven't heard again from Subramanian feel free to get started. Thanks.

Comment 10

[manisai.valluri]

Sure, I will take it up and soon send the fix.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Manisai, I wanted to check back if you already had the time to start working on this bug. If there are remaining issues please let me know.

Comment 12

[nafees87n]

Attachment: Bug 1710356 - Removed hardcoded product name. r=whimboo

Removed hardcoded product name as firefox in "Browser.getVersion in remote/cdp/domains/parent/Browser.jsm

Comment 13

[nafees87n]

Hi Henrik,
I submitted a patch. It is my first contribution. Do let me know if its alright

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thank you Nafees87n, and great to see that you got your first patch up! I had a look at it and added some comments on the Phabricator review. As best lets continue the discussion over there. With the update of your patch done we should be close to get it landed. Thanks again!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a95452a9f995
Removed hardcoded product name. r=whimboo,webdriver-reviewers

Comment 16

[Noemi Erli[:noemi_erli]]

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