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

Properly implement the browscap matching algorithm. #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

csilvers
Copy link
Contributor

https://github.com/browscap/browscap/wiki/Specification:-Lookup-Algorithm says:

If there are multiple matching patterns the longest pattern
should be used to identify the browser. If there are multiple
patterns of the longest length then the pattern that is
earliest in the list of patterns should be used to identify the
browser.

The old code, as far as I can tell -- which isn't very far, to be fair --
always picked the earliest in the list of patterns, regardless of length.
I did the minimal changes to implement the algorithm as specified.
This possibly saved some space in the bargain, as I lowered the space
required for scores from 64 bits to 32.

I added a test-case that failed with the old code but passes with the
new. It requires an up-to-date browscap.ini file, so I updated the
one in test-data as part of this diff.

https://github.com/browscap/browscap/wiki/Specification:-Lookup-Algorithm says:

> If there are multiple matching patterns the longest pattern
> should be used to identify the browser.  If there are multiple
> patterns of the longest length then the pattern that is
> earliest in the list of patterns should be used to identify the
> browser.

The old code, as far as I can tell -- which isn't very far, to be fair --
always picked the earliest in the list of patterns, regardless of length.
I did the minimal changes to implement the algorithm as specified.
This possibly saved some space in the bargain, as I lowered the space
required for scores from 64 bits to 32.

I added a test-case that failed with the old code but passes with the
new.  It requires an up-to-date browscap.ini file, so I updated the
one in test-data as part of this diff.
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.

1 participant