Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Remap Safari 4.1 -> 5 and 6.1/6.2 -> 7 #1244

Closed
wants to merge 2 commits into from
Closed

Conversation

queengooborg
Copy link
Collaborator

This PR adds some logic into the UA parser to remap Safari 4.1 to 5, as well as 6.1/6.2 to 7. This is based upon determining that these releases are backported versions of the new major releases for older macOS systems.

More details for each in the following issues on BCD:
mdn/browser-compat-data#4679 (Safari 4.1)
mdn/browser-compat-data#9423 (Safari 6.1/6.2)

Copy link
Owner

@foolip foolip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you! A few thoughts before we merge.

First, while it's a pretty safe assumption that support in 6.1 implies support in 7, the reverse is not true. Some things might have been OS-dependent, and we don't want to end up claiming something wasn't supported in Safari 7 based on testing in 6.1. (It's a pedantic distinction, but being pedantically correct is kind of the point here.)

Second, will this affect the file names as well? It would be great if it's still pretty obvious where the data came from. If it's not possible to affect one but not the other, maybe this remapping should be an option that we enable in the update script, but disable when generating the filename?

compareVersions.compare(data.version, '4.1', '>=') &&
compareVersions.compare(data.version, '5', '<')
) {
// Safari 4.1 is a backported version of Safari 5
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also link to https://en.wikipedia.org/wiki/Safari_version_history#Safari_4 + https://en.wikipedia.org/wiki/Safari_version_history#Safari_5 as evidence? The release date and WebKit version is the same for 4.1 and 5.0.

compareVersions.compare(data.version, '6.1', '>=') &&
compareVersions.compare(data.version, '7', '<')
) {
// Safari 6.1/6.2 are backported versions of Safari 7
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Safari 6.1/6.2 are backported versions of Safari 7
// Safari 6.1/6.2 are backported versions of Safari 7.0/7.1

Also can you link to https://en.wikipedia.org/wiki/Safari_version_history#Safari_6 + https://en.wikipedia.org/wiki/Safari_version_history#Safari_7 as above?

@queengooborg
Copy link
Collaborator Author

For the first, I doubt there are any differences in support between 6.1 and 7.0, more specifically nothing that BCD would track (such as UI elements). That was the case between Safari 4.1 and 5.0. However, I'd be happy to give this a double-check with collector results!

For the second, the filenames will remain untouched, good news. When saving the files, the collector will generate the filename based upon the absolute version number, so we can easily distinguish the different Chrome builds, or even cases like this.

@foolip
Copy link
Owner

foolip commented Jun 20, 2021

I'd be happy to give this a double-check with collector results!

Do you mean you have access to Safari 7.0 and 7.1? In that case I wonder if shouldn't just treat 6.1 and 6.2 as unmatched versions and not use the results from them at all.

@foolip
Copy link
Owner

foolip commented Jun 22, 2021

Do you mean you have access to Safari 7.0 and 7.1?

Sorry this is not a good question, since we consider both to be "7" in BCD.

@foolip
Copy link
Owner

foolip commented Jun 22, 2021

We need to figure out what 7.1 means here too.

@foolip
Copy link
Owner

foolip commented Jun 22, 2021

As a quickfix to avoid getting bad data into BCD, I'm thinking that for Safari and iOS we should require both major and minor versions to match and treat anything else as unknown. Then rather than excluding certain mappings we've learned are backports, add in the mappings we know are OK in addition to the exact matches.

This would also solve the problem of iOS 14.5 mapping to 14 before we've added 14.5.

@queengooborg
Copy link
Collaborator Author

Closing this in favor of #1572 which ignores the backported versions instead!

@queengooborg queengooborg deleted the backported-safari branch October 15, 2021 16:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants