Check mobile ie11 and ie11 running compatibility mode #34
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisbutcher
approved these changes
Oct 19, 2021
CelineBen
approved these changes
Oct 20, 2021
lib/browser_sniffer.rb
Outdated
| @@ -24,6 +24,14 @@ def ie11? | |||
| browser == :ie && major_browser_version == 11 | |||
| end | |||
|
|
|||
| def ie11_actual? | |||
| ie_11 = browser == :ie && major_browser_version == 11 | |||
There was a problem hiding this comment.
Could we use the ie11? method instead of redefining it?
There was a problem hiding this comment.
I thought about that, but there could be legitimate uses where ie11? is being relied on to return false even if it's ie11 rendering as ie7 in compatibility mode, for example. I don't want to break any apps out there.
There was a problem hiding this comment.
Oh no, I meant using the existing method ie11? instead of defining this ie_11 variable here.
There was a problem hiding this comment.
Ah I see that you have already done that! 👍
Busfox
force-pushed
the
add-full-check-for-ie11
branch
from
2acece4
to
c40a458
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The method
ie11?will return false for ie11 mobile and when ie11 is running in compatibility mode for an older browser.For example:
^ this UA string is for IE11 on windows 10 running in IE7 compatibility mode.
.ie11?will return false, since it's seeing the browser as IE7.^ this UA string is for IE11 mobile running on Windows Phone 8.1.
ie11?will return false since the browser isn't being properly captured.This PR adds a new method called
ie11_actual?that looks a little deeper to determine if the UA is for IE11. It checks:browseris:ieand themajor_browser_versionis 11 - the same check in theie11?methodOR
engine_nameis"Trident"and themajor_engine_versionis7, catching any browsers using the IE11 engineOR
major_browser_versionis11and thebrowser_nameis"IEMobile"Why this approach?
In the case that
1.apps using theie11?method rely on having it returntruefor the browser version being served regardless of whether it's a compatibility view and2.apps using theie11?method rely on only desktop version to causetrueto be returned, I opted to create a new method that will return true when ie11 is in use in any way.