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

Improving QQBrowser detection, separating platforms some to reduce pattern count #1496

Merged
merged 2 commits into from Jun 9, 2017

Conversation

jaydiablo
Copy link
Contributor

Fixes #1495.

Adding platforms/versions/patterns for QQ Browser to improve detection and eliminate some mis-detections I was seeing.

As mentioned in the issue, I did split the files up since the version ranges seem to differ from platform to platform. For the low end of the versions, I tried to only inlucde what I’ve seen with UAs.

I was able to reduce the pattern count some by doing this, here’s line count and pattern count for master today:

$ wc -l ../browscap/build/browscap-ua-test-1496937604/build/full_php_browscap.ini
 2367990 ../browscap/build/browscap-ua-test-1496937604/build/full_php_browscap.ini

$ ack -c "\[.+\]" ../browscap/build/browscap-ua-test-1496937604/build/full_php_browscap.ini
214169

And here’s this branch (based from master):

$ wc -l ../browscap/build/browscap-ua-test-1496937991/build/full_php_browscap.ini
 2349461 ../browscap/build/browscap-ua-test-1496937991/build/full_php_browscap.ini

$ ack -c "\[.+\]" ../browscap/build/browscap-ua-test-1496937991/build/full_php_browscap.ini
212690

Also, here’s a browser name comparison between master and this branch for about 1,600 UAs I collected that should be QQBrowser:

+------------------------+---------------------+----------------------------+
|  Expected Browser Name | browscap-php-3-full | browscap-php-3-full-master |
+------------------------+---------------------+----------------------------+
| qqbrowser (1612)       | qqbrowser (1612)    | qqbrowser (1582)           |
|                        |                     | chrome (18)                |
|                        |                     | android (6)                |
|                        |                     | safari (6)                 |
+------------------------+---------------------+----------------------------+

@codecov
Copy link

codecov bot commented Jun 8, 2017

Codecov Report

Merging #1496 into master will decrease coverage by 0.02%.
The diff coverage is 32.52%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1496      +/-   ##
============================================
- Coverage     36.99%   36.97%   -0.03%     
  Complexity      816      816              
============================================
  Files          1507     1513       +6     
  Lines         56801    56878      +77     
  Branches      23637    23679      +42     
============================================
+ Hits          21015    21030      +15     
- Misses        35786    35848      +62
Impacted Files Coverage Δ Complexity Δ
...ents/browsers/mqq-browser/mqq-browser-android.json 62.24% <0%> (ø) 0 <0> (?)
...ents/browsers/mqq-browser/mqq-browser-generic.json 3.7% <0%> (-2.65%) 0 <0> (ø)
...gents/browsers/mqq-browser/mqq-browser-hd-ios.json 50% <50%> (ø) 0 <0> (?)
...r-agents/browsers/mqq-browser/mqq-browser-ios.json 59.25% <59.25%> (ø) 0 <0> (?)
...nts/browsers/mqq-browser/mqq-browser-mac-2-on.json 80% <80%> (ø) 0 <0> (?)
...ents/browsers/mqq-browser/mqq-browser-mac-1-x.json 80% <80%> (ø) 0 <0> (?)
...ents/browsers/mqq-browser/mqq-browser-windows.json 90.9% <90.9%> (ø) 0 <0> (?)
...ces/user-agents/browsers/edge-browser/edge-12.json 75% <0%> (-5%) 0% <0%> (ø)
...ser-agents/browsers/edge-browser/edge-generic.json 20% <0%> (ø) 0% <0%> (?)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6c6cf4...9763aea. Read the comment docs.

]
},
{
"match": "Mozilla/5.0 (#PLATFORM#) applewebkit* (*khtml*like*gecko*) Version/6.0 MQQBrowser/#MAJORVER#.#MINORVER#* Mobile* Safari*",
Copy link
Member

Choose a reason for hiding this comment

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

Was it intended to leave the version token Version/6.0 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah probably not, it was copied from the iPhone pattern, which did contain that at the time.

I'll remove.

@mimmi20 mimmi20 self-assigned this Jun 9, 2017
@mimmi20 mimmi20 added this to the 6024 milestone Jun 9, 2017
@mimmi20 mimmi20 merged commit 7bfccd5 into browscap:master Jun 9, 2017
@jaydiablo jaydiablo deleted the issue-1495 branch June 10, 2017 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants