-
Notifications
You must be signed in to change notification settings - Fork 9
Remap Safari 4.1 -> 5 and 6.1/6.2 -> 7 #1244
Conversation
There was a problem hiding this 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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 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?
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. |
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. |
Sorry this is not a good question, since we consider both to be "7" in BCD. |
We need to figure out what 7.1 means here too. |
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. |
Closing this in favor of #1572 which ignores the backported versions instead! |
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)