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

Sniff/fixing has("safari") on android stock browser #33

Closed
wants to merge 3 commits into from

Conversation

tkrugg
Copy link
Member

@tkrugg tkrugg commented Jan 16, 2015

This fixes #24.
I also added an automated test that confronts the output of has with the capabilities data.

NOTE: I wasn't able to obtain a valid travis build due to the tests on sauce lab constantly timouting.
I did test it on saucelab though, one or a couple of browsers at a time, and it worked. So i'll just wait and try again later.
In the mean time, feedback is welcome.

@wkeese
Copy link
Member

wkeese commented Jan 16, 2015

The change to sniff.js looks reasonable to me. I don't understand why you are adding a functional test, as opposed to the unit test that already exists. Why not just expand the unit test?

@tkrugg
Copy link
Member Author

tkrugg commented Jan 16, 2015

yes, I should have mentioned this: I wasn't able to retrieve the device information in a unit test: there is no remote.environmentType.

@wkeese
Copy link
Member

wkeese commented Jan 16, 2015

For unit tests, you can get the platform by doing the same kind of test as sniff.js does. I guess you wanted to avoid that since it's not a very good test.

Well, at the least it seems like you can just add a few lines of code to sniff.html, and then write the functional test to load sniff.html, rather than duplicating boilerplate.js and creating a new sniff-auto.html file.

- removed useless boilerplate files
- Updated html file name inside functional tests sniff.js
@tkrugg
Copy link
Member Author

tkrugg commented Jan 19, 2015

I did the changes you asked:

@wkeese
Copy link
Member

wkeese commented Jan 20, 2015

Looks good to me too, I pushed it as 99d0f4e.

@wkeese wkeese closed this Jan 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

decor/sniff: has("safari") returns a non falsy value on android stock browser
2 participants