-
Notifications
You must be signed in to change notification settings - Fork 29
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
FED-2880 Fix Blink-based Microsoft Edge Browser Detection #79
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
expect([browser.isChrome, browser.isEdgeChrome], everyElement(isTrue), | ||
reason: | ||
'Blink-based Edge browser should be detected as both Chrome and Edge ' | ||
'since feature support is based on Chrome, but should identify as Edge ' | ||
'for logging purposes'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test ensures that any consumer's pre-existing feature-based business logic keying off of isChrome
remains unchanged for Edge browsers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+10
@Workiva/release-management-pp |
@Workiva/release-management-p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 from RM
Currently, the modern Edge browser gets detected as
Chrome
- which is not ideal for logging purposes.The browser - from a feature-set standpoint - literally IS
Chrome
- so it should remain being detected as such, but its name/version should reflect the application itself - not just the platform it is built atop of.Also boy scouted some missing test coverage for the legacy (MSIE-based) Edge browser.