Skip to content
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

Extract Safari version instead of WebKit version #1127

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

lukasIO
Copy link

@lukasIO lukasIO commented May 16, 2023

Description
This PR changes the version extraction for Safari to look at the actual Safari version instead of the WebKit version.
Minor versions (e.g. 10.3) get parsed as float in order to keep straight forward version comparison as before.
In case there would be a patch version defined in the ua string (e.g. 10.3.4) the patch version gets dropped.

The oldest Safari versions I could find still work with that approach:

  • macOS Safari 4.0 - Mozilla/5.0 (Macintosh; U; Intel Mac 05 X 10_6_8; en-us) ApplewebKit/531.22.7 (KHTML, like Gecko) Version/4.0.5 Safari/531.22.7
  • iOS Safari 7.0 - Mozilla/5.0 (iPhone; CPU iPhone OS 7_1 like Mac OS X) AppleWebKit/537.51.2 (KHTML, like Gecko) Version/7.0 Mobile/11D167 Safari/9537.53

Purpose
fixes #1082

Safari has stopped updating WebKit versions in the user agent string. E.g. Safari 16.3 still shows AppleWebKit/605.1.15.

@fippo
Copy link
Member

fippo commented Jun 15, 2023

🤔 this would be a breaking change (which means losing 30% of users, see #1065)
I don't think the safari/webkit version is important enough for that, I never gated features on it.

@fippo fippo added the breaking change breaking change that requires a major version bump label Jun 27, 2023
@lukasIO
Copy link
Author

lukasIO commented Jul 7, 2023

thanks for the review!

I never gated features on it

maybe I'm misunderstanding what you mean by that, but there's https://github.com/webrtcHacks/adapter/blob/main/src/js/common_shim.js#L356 and #1082 which details an uncaught (by adapter) bug related to the version parsing for Safari 12.

@lukasIO
Copy link
Author

lukasIO commented Apr 25, 2024

@fippo was not including this in 9.0.0 an oversight or are you not planning to merge this altogether?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change breaking change that requires a major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Safari <=12.0 media establishing does not work due to removeExtmapAllowMixed shim not being applied
2 participants