Closed int3h closed 8 years ago
Safari is most definitely supported. The build process produces a Safari extension, and Safari-related issues are fixed when the devs become aware of them. I think what you're really saying here is that the QA process could use some work, which is true.
Unfortunately, since Safari is only available on Apple computers, not everyone has access to a machine they can use to test RES with Safari, so that's likely the main cause of the problem you're noticing here. When you combine that with the fact that automated test coverage is still qutie minimal, and includes no browser-specific tests, it's pretty easy to see how changes like that might slip through the cracks.
FYI, I did a bit of digging, and regarding automated testing with Continuous Integration on Safari, the only viable option I was able to find is BrowserStack. Other solutions either don't support Safari, or only support Safari versions up to the point where Apple stopped supporting Safari on Windows. BrowserStack does have a free option for open source projects, and supports Safari up to version 9 on Mac. BrowserStack also includes integration with Travis.
Still, I imagine getting automated testing up and running for all browsers will definitely be a non-trivial undertaking.
@int3h this is the second rather perniciously worded issue you've submitted recently. I understand you're frustrated, but the passive aggressiveness isn't needed.
For reference, the PR title in #3192 is rather absurdly worded, and there's a fair amount of misunderstanding in it as well. You got the benefit of the doubt because we empathize with your frustration, but combined with this post it seems it's time to address you personally.
So here's the deal with Safari and why there are more problems in that realm:
First, we are still attempting to support Safari, but Apple has rejected one submission (for trademark issues despite the fact that we have a license to use the word "reddit" in the extension), and indefinitely held the other without any response.
Secondly, Safari is becoming progressively harder and harder to develop for because of increasingly frustrating requirements from Apple. The latest one was that we had to install a beta version of OSX just to be able to continue development. This isn't possible for all people who contribute to RES because Mac hardware is expensive, and because in some instances (mine included!) we can't upgrade our work machines to a beta OS just to continue working on a hobby project. It could interfere with dev environments we use for work, etc. This is why support for Safari has been slower - in major part due to Apple continually moving the goalposts on what we must do just to develop a free extension. See also: having to now pay $100, having to now use Xcode even when we're developing a pure, javascript-only extension.
The suggestion in the other thread that Apple rejected RES for Safari because it was in a state that completely didn't work is an insult to the team of people who work their tails off keeping RES working. Of course it worked when I submitted it, and that bug you pointed out didn't exist when we submitted RES to Apple's store.
The team members who replied to you were, quite frankly, more polite than they needed to be given your smart-alec PR title. The title of this issue is also worded in a similar manner and shows a total lack of empathy or even basic politeness.
This sort of behavior is toxic to the open source community. Should I see continued posts from you worded in unproductive, aggressive fashion like this, I will simply block you (and any other users behaving this way) from submitting issues to our repo as it is detrimental to the team working on RES.
This isn't "to protect their little feelings", mind you. This is because rude behavior such as this chips away at peoples' motivation and happiness working on a project. You're asking people to do work, for free, on a product you use -- for free. "Don't be a jerk" isn't a tall order.
You're right that my wording could have been better, and I apologize for that. I don't mean to create a toxic environment by any means, and I do appreciate the work you guys do on RES.
I think my frustration stems from hearing that Safari support was being worked on, and getting my hopes up that RES was always just on the verge of working in Safari again, once Apple approves it. Then I try to install and run the current code myself, and find that it has been broken for three months, on top of other bugs. The frustration comes from being led to believe it was all on Apple, but then seeing the code has a long way to go on Safari, and nobody seems to be working on it.
I also sincerely didn't mean to insult anybody by saying Apple rejected the extension because of the fatal bug I fixed in the PR. My thinking was that Apple's communication is so terrible, they probably would just not say anything, rather than tell you they couldn't get it to work. Since the fatal bug had gone unnoticed for three months, and I didn't know when the last re-submission of the extension to Apple was, I (incorrectly) assumed that the broken version could have been submitted without anybody realizing it, and without Apple informing you of it.
I definitely recognize the difficulties in Safari extension development. The $99 developer program requirement is really balling any way you look at it, and if you're running Windows, there's no good way to test with Safari directly. Still, if you are trying to support Safari, then some sort of solution has to be found? If not, Safari will keep breaking, to the frustration of Safari users of RES.
Again, I apologize for my tone, and I do appreciate all the hard work put into RES. I will be more respectful with my issues/PRs. Hopefully by submitting one working PR that fixed a major Safari bug, and tracking down another to the specific lines and commits, I have shown my good faith and willingness to help. I'll try to make my wording reflect that better.
I think my frustration stems from hearing that Safari support was being worked on, and getting my hopes up that RES was always just on the verge of working in Safari again, once Apple approves it. Then I try to install and run the current code myself, and find that it has been broken for three months, on top of other bugs. The frustration comes from being led to believe it was all on Apple, but then seeing the code has a long way to go on Safari, and nobody seems to be working on it.
Believe me, I understand your frustration. Unfortunately very few of us are able to work on RES for Safari now due to their new and annoying barrier to entry. Before we push anything to the store, we DO get Safari working and test things at a cursory level. To be fair, we may miss a test here or there because Safari seems to be the only browser anymore with frustrating/obscure implementation quirks (such as the drag to resize one) that are unique only to that browser.
The master branch should always be expected to have some bugs, on all browsers, because it's always in flux. We push code and break things all the time because not every contributor can test on every browser.
I appreciate your willingness to look inward and acknowledge your previous tone / choice of words. It is my goal as the owner of this project to foster an environment that encourages people to contribute, so it's important to me that I do everything I can to make sure that people are not discouraged from helping. Thank you for understanding that, it sincerely has made my day.
I don’t know exactly how much free time I have these days, but is there a specific/structured way I can help contribute to better Safari support?
I don’t know if I’ll be able to own code, but I could definitely at least build+install+run a revision in Safari, to do a basic sanity test. If there is a documented set of tasks I can perform to test that everything works (human unit testing?), I would be happy to run through those. If not, I can just install and use the revision to be a super beta tester, who can drill down to the code level when I find bugs.
If I may also be so bold, I'd like to also suggest some project changes that could help improve development, including cross-platform development. I mean no offense by these suggestions, and I am definitely not demanding you guys adopt them or anything. They are just approaches to development I've seen that have helped combat the very problem RES is currently having.
As was pointed out, a CI server that can test using Safari is hard to get. However, there's a middle ground between a fully automated CI server and no tests at all. If RES had a set of tests that could be manually run in the browser, and report their results on screen, that would make "pull and install latest version and check for regressions in Safari" much easier.
Instead of just having a tester open http://reddit.com and then clicking around on some stuff, you could have them open "tests.html", which would run the tests in their browser. Then I (or any other tester) could manually report test failures, with details, back on GitHub. Doing this would mean you could ignore the automation part of testing, and just focus on tests on their own, though that is its own challenge.
I know better than many that adding tests to a large codebase written without testing in mind is hard, so I definitely don’t expect that to be a near term change. However, it seems like it may be the right goal in order to decrease developer workload, increase development velocity, and increase stability, especially on a large, cross-browser project like this.
A slightly more manageable change that could help development would be to adopt some sort of structured development workflow, like GitHub Flow. Changes/new features would be gated by Pull Requests, which would get both automatic (CI, etc.) review and manual (code style, manual cross-browser testing) review before being approved and merged. Something like lgtm can help in manual code review/browser testing.
With the current “everything on master” approach, it’s hard to see all the changes made by a particular feature/task/chore, and so hard to isolate and test them for bugs. I can check out master
every week or so and run it in Safari, but I won’t be able to see why user tagging (for example) broke without doing some code archeology. If RES used something like GitHub Flow, I could checkout a PR for “optimize image interaction” or whatever, and run it in Safari, and then tell the developer “the idea is good, but it breaks in Safari.” The PR would then be kept out of master until it was fixed. In this way, regressions could be caught during development, before making it into master.
A tool like lgtm can help formalize manual code review and testing.
The current process of one PR per project has been working pretty well for code review and Travis/AppVeyor checking javascript/css linting and a decent chunk of unit tests. Git-flow turned out to add a lot of extra work for erikdesjardins and me,so right now it feels better for RES to use master, project branches, "stable/release" tags, akin to https://github.com/honestbleeps/Reddit-Enhancement-Suite/issues/2549#issuecomment-165568744 -- especially when releases start shipping more often.
Lgtm sounds like a good idea when res might get more devoted manual testing, but right now would probably be more annoying than helpful. Might be worth it for eyes on changes, though.
Automated selenium testing would be great, but certainly needs lots of setup and test-writing. Any more ideas to add to https://github.com/honestbleeps/Reddit-Enhancement-Suite/issues/2587 ?
A manual test suite does sound like a good middle ground. Any suggestions for frameworks/webapps to help organize test plans and results?
It's definitely useful for people to pull from master and build for themselves, then report bugs you notice via Github including the browser version and git commit hash. Bisecting is going above and beyond 🙌 obvious disclaimer: there will be bugs, they night not get fixed immediately.
@int3h Sometimes the best you can start with is just to use it. That's how I started with RES - Now, I'm not particularly a developer, I just respond to questions in RESIssues mostly (and I like to pat myself on the back by starting the drive toward build automation, but I only tell myself that to sleep soundly at night)
This stands true with any open source project. Do what you can to test, prod at, poke, and use development versions for safari. After that it all just kinda comes naturally as for what needs to be done, where things need to be focused, etc.
This is all easier said then done, but its the advice I offer anyone who wants to contribute to any open source project I am apart of. Just start playing with it. You'll know where to go from there.
Safari is not actually supported by the dev team in any way. As far as I can tell, no core dev runs Safari, even for testing purposes. Further, no reference sources are consulted when making code changes to see if they are Safari compatible.
As evidence, see #3192 and #3209. The former completely broke the extension in Safari for over three months because of a blatant bug in Safari-specific code, and no developer noticed; the latter broke drag-to-zoom on images due to relying on Safari incompatible mouse event attributes.
MDN documents cross-platform support for nearly every web feature, and a quick glance at
MouseEvent.buttons
, for example, shows that it is not supported in Safari. Checking this could have prevented breaking click-to-zoom in Safari.Looking at @larsjohnsen's recent commits, you'll see "checked on Chrome, Firefox and Edge", and no sign of Safari. (Not picking on him, but using him as an example, since he committed the Safari incompatible code in click-to-zoom.) There is no CI server or other automated testing infrastructure to catch the regression either, for devs that don't have Macs. And apparently, no other dev ever thinks to start up Safari and install RES to check if it still works by hand.
If RES supports Safari, then support Safari. If, on the other hand, Safari isn't considered during development, or even run, then stop saying RES supports Safari.