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

Add Safari 4 as ranged version #6915

Merged
merged 6 commits into from
Nov 4, 2020

Conversation

queengooborg
Copy link
Collaborator

@queengooborg queengooborg commented Oct 14, 2020

This PR fixes #6807. Due to the challenges of testing Safari 1 through 2, and the lack of Safari 3 in BrowserStack, this PR adds a range for Safari 3 and lower, as well as Safari 4 and lower.

Explanation of challenges:
Safari 1 and 2 were introduced for Mac OS X 10.2 and 10.3, both of which are built upon PowerPC architecture. As a result, modern emulators do not support running such old editions of Mac OS X. I was able to find a program that emulates PowerPC architecture, though it was been in beta for over a decade. There is no sensible way to send files to the PowerPC VMs (you need a Mac or Mac image reader software like Paragon HFS+), and although I was able to set up an internet connection, it's spotty at best, and requires disabling and re-enabling the network connection for the host VM every time the host VM boots up, then resetting the PowerPC VM's connection a few times.

Bottom line is...it's inefficient, borderline impossible, to test Safari 1-2. While I do plan on adding exact data for Safari 1-2, it would be much easier to run through all of those features at once at a later date, and simply set a ranged value for the time being. (This will also give me some extra time to try configuring another emulator while we achieve our goals.)

Edit: Since Safari 3 is not available on BrowserStack or SauceLabs, this PR includes a range for Safari 4 instead.

@github-actions github-actions bot added the linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files. label Oct 14, 2020
@foolip
Copy link
Collaborator

foolip commented Oct 14, 2020

I think we should do this! @vinyldarkscratch for Safari 3, is that available for macOS running on x86, or are only the Windows versions convenient to test for some range?

@queengooborg
Copy link
Collaborator Author

Unfortunately Safari 3.x for macOS is also for versions that run on PowerPC architecture, so at the moment, it's only sensible to test in Windows.

@foolip
Copy link
Collaborator

foolip commented Oct 15, 2020

What is the earliest version of Safari that's practical to test on macOS, available on either BrowserStack or Sauce? That would be a useful cutoff point for more contributors, and if it's also very far back in time, the difference doesn't matter much.

@sideshowbarker
Copy link
Collaborator

+1 👍 I also think we should do this.

@foolip
Copy link
Collaborator

foolip commented Oct 15, 2020

To answer my own question, Safari 4.0.5 on Mac OS X 10.6.8 is the oldest available on BrowserStack. Sauce's oldest is Safari 8.

So I'd suggest we add ≤4, as that's something more people could verify.

@queengooborg queengooborg changed the title Add Safari 3 as a ranged version Add Safari 4 as a ranged version Oct 15, 2020
@queengooborg queengooborg changed the title Add Safari 4 as a ranged version Add Safari 3+4 as ranged versions Oct 15, 2020
@queengooborg
Copy link
Collaborator Author

I added a range for Safari 4 as well, good point! Since I'm able to test as far back as Safari 3 efficiently, I thought it would good to add them both, since we can get closer to real values that way? (Happy to keep it as just ≤4 though!)

@queengooborg queengooborg changed the title Add Safari 3+4 as ranged versions Add Safari 4 as ranged version Oct 28, 2020
@queengooborg
Copy link
Collaborator Author

Verbal discussion with @foolip led to the conclusion to simply keep Safari 4 as the only ranged version.

Copy link
Collaborator

@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 looks good to me, but I shouldn't merge myself, deferring to @ddbeck.

@foolip
Copy link
Collaborator

foolip commented Nov 3, 2020

Friendly ping, @ddbeck, this is blocking @vinyldarkscratch and me making progress on Safari PRs.

@ddbeck
Copy link
Collaborator

ddbeck commented Nov 3, 2020

@foolip Thanks for the ping (and your patience on this).

I'm OK with this, but https://github.com/mdn/browser-compat-data/blob/master/schemas/compat-data-schema.md#ranged-versions needs to be updated as well.

@foolip
Copy link
Collaborator

foolip commented Nov 3, 2020

@vinyldarkscratch over to you :)

@github-actions github-actions bot added the schema ⚙️ Isses or pull requests regarding the JSON schema files used in this project. label Nov 3, 2020
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

This looks good. I do have one suggestion for the docs but, in the interest of unblocking this, I'm going to directly apply and merge it. Thank you!

schemas/compat-data-schema.md Outdated Show resolved Hide resolved
@ddbeck ddbeck merged commit 320b6fe into mdn:master Nov 4, 2020
@queengooborg queengooborg deleted the linter/safari-ranged-version branch November 4, 2020 21:09
@foolip
Copy link
Collaborator

foolip commented Nov 5, 2020

Wohoo, thanks @ddbeck! @vinyldarkscratch looking forward to turning Safari nulls and trues into something else! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files. schema ⚙️ Isses or pull requests regarding the JSON schema files used in this project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add range for Safari ≤3?
4 participants