[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user klopfdreh closed the pull request at: https://github.com/apache/wicket/pull/275 ---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r184343253
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -133,24 +140,99 @@ public final String getUserAgent()
}
/**
-* returns the user agent string (lower case).
+* Initializes the {@link WebClientInfo} user agent detection. This can
be overridden to choose
+* a different detection as YAUAA
(https://github.com/nielsbasjes/yauaa) - if you do so, you
+* might exclude the maven dependency from your project in favor of a
different framework.
+*/
--- End diff --
Ok, then keep the dependency and just override it. We have to adjust the
documentation here.
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user svenmeier commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r184288167
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -133,24 +140,99 @@ public final String getUserAgent()
}
/**
-* returns the user agent string (lower case).
+* Initializes the {@link WebClientInfo} user agent detection. This can
be overridden to choose
+* a different detection as YAUAA
(https://github.com/nielsbasjes/yauaa) - if you do so, you
+* might exclude the maven dependency from your project in favor of a
different framework.
+*/
--- End diff --
No, you can't exclude the dependency - with UserAgentAnalyzer in the
signature of #detectBrowserProperties(UserAgentAnalyzer) you will get a
NoClassDefFoundError if the dependency is not present.
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r182993206
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
Sorry for delay, here is the code works for me:
```
private static ThreadLocal localAnalyzer = new
ThreadLocal();
@Before
public void before()
{
requestCycleMock = mock(RequestCycle.class);
webRequest = mock(ServletWebRequest.class);
when(requestCycleMock.getRequest()).thenReturn(webRequest);
servletRequest = mock(HttpServletRequest.class);
when(webRequest.getContainerRequest()).thenReturn(servletRequest);
if (localAnalyzer.get() == null) {
WebClientInfo webClientInfo = new
WebClientInfo(requestCycleMock, "test");
webClientInfo.gatherExtendedInfo();
localAnalyzer.set(Application.get().getMetaData(WebClientInfo.UAA_META_DATA_KEY));
} else {
Application.get().setMetaData(WebClientInfo.UAA_META_DATA_KEY,
localAnalyzer.get());
}
}
@AfterClass
public static void staticAfter()
{
localAnalyzer.set(null);
}
```
Here are the measurment results:
```
Combined tests
real0m24.123s
user1m30.748s
sys 0m1.804s
Individual tests (after above changes were applied)
real0m22.844s
user1m23.948s
sys 0m1.832s
```
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user svenmeier commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181308489
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -46,14 +46,20 @@
/**
* The user agent string from the User-Agent header, app.
Theoretically, this might differ from
-* {@link
org.apache.wicket.protocol.http.ClientProperties#isNavigatorJavaEnabled()}
property, which is
-* not set until an actual reply from a browser (e.g. using {@link
BrowserInfoPage} is set.
+* {@link
org.apache.wicket.protocol.http.ClientProperties#isNavigatorJavaEnabled()}
property,
+* which is not set until an actual reply from a browser (e.g. using
{@link BrowserInfoPage} is
+* set.
*/
private final String userAgent;
/** Client properties object. */
private final ClientProperties properties;
+ private final static UserAgentAnalyzer UAA =
UserAgentAnalyzer.newBuilder()
--- End diff --
UserAgentAnalyzer#parse(UserAgent) is synchronized, please check its code.
This is even mentioned on the project home page.
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181303063
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
Hehe me too ð - no hurry, because I think this is going to be released
in a further version.
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181297750
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
Will try to provide patch later tonight
Have to do day time job right now :(
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181296712
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
May you have any other good idea to keep the tests individual.
Just send a patch to me if something would fit the requirement and I am
going to integrate it.
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181295733
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
Yes that is true, but I think if you change the code / the browser
detection you are going to investigate why the test is failing and fix the
integration as long as every test is back working again.
@BeforeClass is not working, because WicketTestCase and at this stage no
Application is attached to the main thread.
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181294275
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
All subsequent lines will not be executed in case of failure
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181293918
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
You can move init to `@BeforeClass` .
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181293718
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
You will see if something fails in the test results btw.
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181293658
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
This might slow down the test execution because of the init of YAUAA - try
it out.
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181293591
--- Diff:
wicket-core/src/test/java/org/apache/wicket/protocol/http/request/WebClientInfoTest.java
---
@@ -59,9 +58,52 @@ public void before()
}
/**
-* Test IE 6.x user-agent strings
+* Test check check all user agents
*/
@Test
+ public void testBrowsersAndVersionsOfUserAgents() {
+ // Internet Explorers
--- End diff --
I would rather have all these as individual tests, to be able to see which
one was failed
And run all of them regardless failure state of others
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181293350
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -147,6 +149,93 @@ private String getUserAgentStringLc()
return (getUserAgent() != null) ? getUserAgent().toLowerCase()
: "";
}
+ /**
+* Initializes the {@link WebClientInfo} user agent detection. This can
be overridden to choose
+* a different detection as YAUAA
(https://github.com/nielsbasjes/yauaa) - if you do so, you
+* might exclude the maven dependency from your project in favor of a
different framework.
+*/
+ public void initialize()
+ {
+ UserAgentAnalyzer userAgentAnalyzer =
Application.get().getMetaData(UAA_META_DATA_KEY);
+ if (userAgentAnalyzer == null)
+ {
+ userAgentAnalyzer = UserAgentAnalyzer.newBuilder()
+ .hideMatcherLoadStats()
+ .withCache(25000)
+ .build();
+ Application.get().setMetaData(UAA_META_DATA_KEY,
userAgentAnalyzer);
+ }
+ detectBrowserProperties(userAgentAnalyzer);
+ }
+
+ /**
+* Detects browser properties like versions or the type of the browser
and applies them to the
+* {@link ClientProperties}, override this method if there are errors
within the browser /
+* version detection due to newer browsers
+*
+* @param userAgentAnalyzer
+*the user agent analyzer to detect browsers and versions
+*/
+ protected void detectBrowserProperties(UserAgentAnalyzer
userAgentAnalyzer)
+ {
+
+ nl.basjes.parse.useragent.UserAgent parsedUserAgent =
userAgentAnalyzer
+ .parse(getUserAgent());
+ String userAgentName = parsedUserAgent.getValue("AgentName");
+
+ // Konqueror
+
properties.setBrowserKonqueror(UserAgent.KONQUEROR.getUaStrings().contains(userAgentName));
+
+ // Chrome
+
properties.setBrowserChrome(UserAgent.CHROME.getUaStrings().contains(userAgentName));
+
+ // Edge
+
properties.setBrowserEdge(UserAgent.EDGE.getUaStrings().contains(userAgentName));
+
+ // Safari
+
properties.setBrowserSafari(UserAgent.SAFARI.getUaStrings().contains(userAgentName));
+
+ // Opera
+
properties.setBrowserOpera(UserAgent.OPERA.getUaStrings().contains(userAgentName));
+
+ // Internet Explorer
+ properties.setBrowserInternetExplorer(
+
UserAgent.INTERNET_EXPLORER.getUaStrings().contains(userAgentName));
+
+ // FireFox
+ boolean isFireFox = UserAgent.FIREFOX.getUaStrings()
+ .contains(parsedUserAgent.getValue("AgentName"));
+ if (isFireFox)
+ {
+ properties.setBrowserMozillaFirefox(true);
+ properties.setBrowserMozilla(true);
+ }
+ else
+ {
+ properties.setBrowserMozilla(
+
UserAgent.MOZILLA.getUaStrings().contains(parsedUserAgent.getValue("AgentName")));
--- End diff --
`userAgentName`
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181293318
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -147,6 +149,93 @@ private String getUserAgentStringLc()
return (getUserAgent() != null) ? getUserAgent().toLowerCase()
: "";
}
+ /**
+* Initializes the {@link WebClientInfo} user agent detection. This can
be overridden to choose
+* a different detection as YAUAA
(https://github.com/nielsbasjes/yauaa) - if you do so, you
+* might exclude the maven dependency from your project in favor of a
different framework.
+*/
+ public void initialize()
+ {
+ UserAgentAnalyzer userAgentAnalyzer =
Application.get().getMetaData(UAA_META_DATA_KEY);
+ if (userAgentAnalyzer == null)
+ {
+ userAgentAnalyzer = UserAgentAnalyzer.newBuilder()
+ .hideMatcherLoadStats()
+ .withCache(25000)
+ .build();
+ Application.get().setMetaData(UAA_META_DATA_KEY,
userAgentAnalyzer);
+ }
+ detectBrowserProperties(userAgentAnalyzer);
+ }
+
+ /**
+* Detects browser properties like versions or the type of the browser
and applies them to the
+* {@link ClientProperties}, override this method if there are errors
within the browser /
+* version detection due to newer browsers
+*
+* @param userAgentAnalyzer
+*the user agent analyzer to detect browsers and versions
+*/
+ protected void detectBrowserProperties(UserAgentAnalyzer
userAgentAnalyzer)
+ {
+
+ nl.basjes.parse.useragent.UserAgent parsedUserAgent =
userAgentAnalyzer
+ .parse(getUserAgent());
+ String userAgentName = parsedUserAgent.getValue("AgentName");
+
+ // Konqueror
+
properties.setBrowserKonqueror(UserAgent.KONQUEROR.getUaStrings().contains(userAgentName));
+
+ // Chrome
+
properties.setBrowserChrome(UserAgent.CHROME.getUaStrings().contains(userAgentName));
+
+ // Edge
+
properties.setBrowserEdge(UserAgent.EDGE.getUaStrings().contains(userAgentName));
+
+ // Safari
+
properties.setBrowserSafari(UserAgent.SAFARI.getUaStrings().contains(userAgentName));
+
+ // Opera
+
properties.setBrowserOpera(UserAgent.OPERA.getUaStrings().contains(userAgentName));
+
+ // Internet Explorer
+ properties.setBrowserInternetExplorer(
+
UserAgent.INTERNET_EXPLORER.getUaStrings().contains(userAgentName));
+
+ // FireFox
+ boolean isFireFox = UserAgent.FIREFOX.getUaStrings()
+ .contains(parsedUserAgent.getValue("AgentName"));
--- End diff --
`userAgentName`
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181286646
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -46,14 +46,20 @@
/**
* The user agent string from the User-Agent header, app.
Theoretically, this might differ from
-* {@link
org.apache.wicket.protocol.http.ClientProperties#isNavigatorJavaEnabled()}
property, which is
-* not set until an actual reply from a browser (e.g. using {@link
BrowserInfoPage} is set.
+* {@link
org.apache.wicket.protocol.http.ClientProperties#isNavigatorJavaEnabled()}
property,
+* which is not set until an actual reply from a browser (e.g. using
{@link BrowserInfoPage} is
+* set.
*/
private final String userAgent;
/** Client properties object. */
private final ClientProperties properties;
+ private final static UserAgentAnalyzer UAA =
UserAgentAnalyzer.newBuilder()
--- End diff --
The initialization is expensive, but the UserAgentAnalyzer is thread safe
so it can be reused. Maybe we are able to use MetaDataKey
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181286230
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -193,165 +198,117 @@ protected String getRemoteAddr(RequestCycle
requestCycle)
*/
private void init()
{
- setInternetExplorerProperties();
- setOperaProperties();
- setMozillaProperties();
- setKonquerorProperties();
- setChromeProperties();
- setEdgeProperties();
- setSafariProperties();
+ nl.basjes.parse.useragent.UserAgent parsedUserAgent =
UAA.parse(getUserAgent());
+ setInternetExplorerProperties(parsedUserAgent);
+ setKonquerorProperties(parsedUserAgent);
+ setMozillaProperties(parsedUserAgent);
+ setOperaProperties(parsedUserAgent);
+ setChromeProperties(parsedUserAgent);
+ setEdgeProperties(parsedUserAgent);
+ setSafariProperties(parsedUserAgent);
--- End diff --
Hi @solomax - yes indeed - it was due to the refactoring and those methods
were present before - I just refactored them.
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181282823
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -193,165 +198,117 @@ protected String getRemoteAddr(RequestCycle
requestCycle)
*/
private void init()
{
- setInternetExplorerProperties();
- setOperaProperties();
- setMozillaProperties();
- setKonquerorProperties();
- setChromeProperties();
- setEdgeProperties();
- setSafariProperties();
+ nl.basjes.parse.useragent.UserAgent parsedUserAgent =
UAA.parse(getUserAgent());
+ setInternetExplorerProperties(parsedUserAgent);
+ setKonquerorProperties(parsedUserAgent);
+ setMozillaProperties(parsedUserAgent);
+ setOperaProperties(parsedUserAgent);
+ setChromeProperties(parsedUserAgent);
+ setEdgeProperties(parsedUserAgent);
+ setSafariProperties(parsedUserAgent);
--- End diff --
can be replaced with
String agent = parsedUserAgent.getValue("AgentName");
properties.setBrowserKonqueror(UserAgent.KONQUEROR.getUaStrings().contains(agent));
properties.setBrowserChrome(UserAgent.CHROME.getUaStrings().contains(agent));
.
setBrowserVersion(parsedUserAgent);
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181282313
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -193,165 +198,117 @@ protected String getRemoteAddr(RequestCycle
requestCycle)
*/
private void init()
{
- setInternetExplorerProperties();
- setOperaProperties();
- setMozillaProperties();
- setKonquerorProperties();
- setChromeProperties();
- setEdgeProperties();
- setSafariProperties();
+ nl.basjes.parse.useragent.UserAgent parsedUserAgent =
UAA.parse(getUserAgent());
+ setInternetExplorerProperties(parsedUserAgent);
+ setKonquerorProperties(parsedUserAgent);
+ setMozillaProperties(parsedUserAgent);
+ setOperaProperties(parsedUserAgent);
+ setChromeProperties(parsedUserAgent);
+ setEdgeProperties(parsedUserAgent);
+ setSafariProperties(parsedUserAgent);
--- End diff --
All these methods looks very much the same
---
[GitHub] wicket pull request #275: [WICKET-6544] mobile browser detection is improved...
Github user svenmeier commented on a diff in the pull request:
https://github.com/apache/wicket/pull/275#discussion_r181224559
--- Diff:
wicket-core/src/main/java/org/apache/wicket/protocol/http/request/WebClientInfo.java
---
@@ -46,14 +46,20 @@
/**
* The user agent string from the User-Agent header, app.
Theoretically, this might differ from
-* {@link
org.apache.wicket.protocol.http.ClientProperties#isNavigatorJavaEnabled()}
property, which is
-* not set until an actual reply from a browser (e.g. using {@link
BrowserInfoPage} is set.
+* {@link
org.apache.wicket.protocol.http.ClientProperties#isNavigatorJavaEnabled()}
property,
+* which is not set until an actual reply from a browser (e.g. using
{@link BrowserInfoPage} is
+* set.
*/
private final String userAgent;
/** Client properties object. */
private final ClientProperties properties;
+ private final static UserAgentAnalyzer UAA =
UserAgentAnalyzer.newBuilder()
--- End diff --
UserAgentAnalyzer isn't reentrant. Since it is expensive to create one,
instances have to be pooled.
---
