Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for Linux-specific UA string #7158

Closed
larsbergstrom opened this issue Aug 11, 2015 · 7 comments
Closed

Add support for Linux-specific UA string #7158

larsbergstrom opened this issue Aug 11, 2015 · 7 comments
Labels
A-platform/utils E-easy

Comments

@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented Aug 11, 2015

In #7143, we added a User Agent string for Servo, but only for Android, Gonk, and OSX. We would like to add a platform-specific Linux modifier.

Based on information from @gerv in #7143 (comment), we will need to include the full set of bits such as X11; Ubuntu; Linux x86_64, which will require some analysis of the environment at startup. These additional bits (distro name) appear to be used for sites that want to be able to suggest the correct download.

CC: @SimonSapin
@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Aug 11, 2015

Is the distro name really needed? My Firefoxes, respectively Release from archlinux.org/packages and Nightly from mozilla.org, currently have:

Mozilla/5.0 (X11; Linux x86_64; rv:39.0) Gecko/20100101 Firefox/39.0
Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0

I wouldn’t be surprised if Ubuntu is a vanity bit added by packagers.
@larsbergstrom
Copy link
Contributor Author

@larsbergstrom larsbergstrom commented Aug 11, 2015

@SimonSapin That seems consistent with https://wiki.mozilla.org/Firefox/User_Agent, though there isn't a ton of info I can find there or in other bugs on BMO.

According to the source (http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#695), the X11 is a historical bit and not used by sites for anything.
@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Aug 11, 2015

https://bazaar.launchpad.net/~mozillateam/firefox/firefox.wily/view/head:/debian/patches/ubuntu-ua-string-changes.patch is an Ubuntu-specific patch adding the Ubuntu bit. The commit message that first added it says "Modify the UA string to add "Ubuntu" to the platform component", but doesn’t say why.
@gerv
Copy link

@gerv gerv commented Aug 18, 2015

Yes, don't include the "Ubuntu" bit. Copy what stock Firefox does, which is "X11; Linux x86_64" or some related string for 32-bit if you guys support that.
@larsbergstrom larsbergstrom added the E-easy label Nov 4, 2015
@larsbergstrom
Copy link
Contributor Author

@larsbergstrom larsbergstrom commented Nov 4, 2015

Marking this as E-Easy, as we can skip the runtime determinination and figure out all of this statically from the compilation target architecture.
@jdm
Copy link
Member

@jdm jdm commented Nov 4, 2015

See default_user_agent_string in opts.rs.
ben0x539 added a commit to ben0x539/servo that referenced this issue Nov 5, 2015
@ben0x539
Make desktop UA string depend on build target.
b84087c 
This implements servo#7158 by conditionally choosing a UA string by
`#[cfg()]`-checking for `target_os = linux` and whether `target_arch` is
`x86_64` or not. Matching the behavior of Firefox, either "X11; Linux
x86_64" or "X11; Linux i686" is included.

`target_os = windows` is also checked; again as in Firefox "Windows NT
6.1; Win64; x64" or just "Windows NT 6.1" is included. The UA string
pretends to be non-WoW64 Windows 7, since there's only so much we can
detect at build time.

The existing desktop UA string that lists OS X is chosen if `target_os`
is neither `linux` nor `windows`.
@ben0x539 ben0x539 mentioned this issue Nov 5, 2015
Merged
bors-servo added a commit that referenced this issue Nov 5, 2015
@bors-servo
Auto merge of #8353 - ben0x539:linux-ua, r=larsbergstrom
Loading status checks…
7efdcbe 
Make desktop UA string depend on build target.

This implements #7158 by conditionally choosing a UA string by
`#[cfg()]`-checking for `target_os = linux` and whether `target_arch` is
`x86_64` or not. Matching the behavior of Firefox, either "X11; Linux
x86_64" or "X11; Linux i686" is included.

`target_os = windows` is also checked; again as in Firefox "Windows NT
6.1; Win64; x64" or just "Windows NT 6.1" is included. The UA string
pretends to be non-WoW64 Windows 7, since there's only so much we can
detect at build time.

The existing desktop UA string that lists OS X is chosen if `target_os`
is neither `linux` nor `windows`.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8353)
<!-- Reviewable:end -->
@jdm
Copy link
Member

@jdm jdm commented Nov 23, 2015

Fixed by #8353.
@jdm jdm closed this Nov 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-platform/utils E-easy
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@jdm @SimonSapin @gerv @larsbergstrom
You can’t perform that action at this time.