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

Check Trident version to detect compatibility mode #8884

Merged
merged 5 commits into from
Mar 22, 2017
Merged

Check Trident version to detect compatibility mode #8884

merged 5 commits into from
Mar 22, 2017

Conversation

Darsstar
Copy link
Contributor

@Darsstar Darsstar commented Mar 20, 2017

IE11 in compatibility mode is actually not too old. It's just not recognized as IE11 if you leave the Trident version out of the equation.
Checking the Trident version is something Vaadin 7 (since 7.0.0.beta5) does: fc15f16
Trident 4 -> IE8
Trident 5 -> IE9
Trident 6 -> IE10
Trident 7 -> IE11 (windows 7, 10)
Trident 8 -> IE11 (windows 10 initial release only)


This change is Reviewable

IE11 in compatibility mode is actually not too old. It's just not recognized as IE11 if you leave the Trident version out of the equation.
Checking the Trident version is something Vaadin 7 (since 7.0.0.beta5) does: fc15f16
Trident 4 -> IE8
Trident 5 -> IE9
Trident 6 -> IE10
Trident 7 -> IE11 (windows 7, 10)
Trident 8 -> IE11 (windows 10 initial release only)
@CLAassistant
Copy link

CLAassistant commented Mar 20, 2017

CLA assistant check
All committers have signed the CLA.

Check the Trident version in a different place and use it to determine the actual IE version instead of trusting the possibly emulated MSIE version string.
@pleku
Copy link
Contributor

pleku commented Mar 21, 2017

Reviewed 1 of 1 files at r1.
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


a discussion (no related file):
Hi! Thanks for the contribution. It would nice to add a test for this in VBrowserDetailsUserAgentParserTest so that it will not get removed again.

If you don't have the time to do that, I can add the test myself, but the main question is: what is the user agent for IE11 in compatibility mode ? Need to try to look that up


Comments from Reviewable

@pleku
Copy link
Contributor

pleku commented Mar 21, 2017

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


shared/src/main/java/com/vaadin/shared/VBrowserDetails.java, line 155 at r2 (raw file):

                    int majorVersion = browserMajorVersion = 11;
                    if (browserEngineVersion <= 7) {
                        majorVersion = (int) (browserEngineVersion + 4);

This is the kind of magic that I should be described in a comment, I know Microsoft works in mysterious ways, but still, where does this come from ?


Comments from Reviewable

@pleku
Copy link
Contributor

pleku commented Mar 21, 2017

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

- IE11 Windows 7 compatibility view (CV) IE7
- IE11 Windows 10 CV IE7
- IE11 launch day Windows 10 CV IE7
@Darsstar
Copy link
Contributor Author

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.


a discussion (no related file):

Previously, pleku (Pekka Hyvönen) wrote…

Hi! Thanks for the contribution. It would nice to add a test for this in VBrowserDetailsUserAgentParserTest so that it will not get removed again.

If you don't have the time to do that, I can add the test myself, but the main question is: what is the user agent for IE11 in compatibility mode ? Need to try to look that up

Added three new tests.

For reference:

When in compatibility mode (they all start reporting IE7 by default)
IE8: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C)
IE9: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.1; Trident/5.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C)
IE10: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.1; Trident/6.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C)
IE11 - Win 7: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 6.1; WOW64; Trident/7.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E)
IE11 - Win 10: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 10.0; WOW64; Trident/7.0; .NET4.0C; .NET4.0E)
IE11 - Win 10 launch day: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 10.0; WOW64; Trident/8.0; .NET4.0C; .NET4.0E)


shared/src/main/java/com/vaadin/shared/VBrowserDetails.java, line 155 at r2 (raw file):

his is the kind of magic that I should be described in a comment, I know Microsoft works in mysterious ways, but still, where does this come from ?
Done


Comments from Reviewable

@pleku
Copy link
Contributor

pleku commented Mar 22, 2017

Reviewed 2 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@pleku pleku merged commit baa0baf into vaadin:master Mar 22, 2017
@Darsstar Darsstar deleted the patch-1 branch March 22, 2017 07:47
@Darsstar
Copy link
Contributor Author

@pleku Will this end up in 8.0.4 or should I have approached making this contribution differently if that was my goal? And if so, what would I need to do to get it merged into 8.0.4 too?

@pleku
Copy link
Contributor

pleku commented Mar 22, 2017

@Darsstar don't worry I'm picking it to 8.0.4

@pleku pleku added this to the 8.0.4 milestone Mar 22, 2017
pleku pushed a commit that referenced this pull request Mar 22, 2017
IE11 in compatibility mode is actually not too old. It's just not recognized as IE11 if you leave the Trident version out of the equation. Checking the Trident version and using it to determine the actual IE version instead of trusting the possibly emulated MSIE version string.
hesara pushed a commit that referenced this pull request Mar 22, 2017
IE11 in compatibility mode is actually not too old. It's just not recognized as IE11 if you leave the Trident version out of the equation. Checking the Trident version and using it to determine the actual IE version instead of trusting the possibly emulated MSIE version string.
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.

None yet

3 participants