mbien commented on code in PR #6626:
URL: https://github.com/apache/netbeans/pull/6626#discussion_r1373509921
##########
ide/extbrowser/src/org/netbeans/modules/extbrowser/NbDdeBrowserImpl.java:
##########
@@ -127,6 +133,59 @@ public NbDdeBrowserImpl (ExtWebBrowser extBrowserFactory) {
*/
public static native String getDefaultOpenCommand() throws
NbBrowserException;
+ /**
+ * Get the default browser name using Java JNA library
+ * @return String
+ */
+ private static String getDefaultWindowsBrowser() {
+ String userChoice = Advapi32Util
+ .registryGetStringValue(
+ WinReg.HKEY_CURRENT_USER,
+
"SOFTWARE\\Microsoft\\Windows\\Shell\\Associations\\UrlAssociations\\https\\UserChoice",
+ "ProgId"
+ )
+ .toUpperCase(Locale.ROOT);
Review Comment:
this will throw a NPE if `registryGetStringValue(...)` returns null
change to:
```java
String userChoice = Advapi32Util
.registryGetStringValue(
WinReg.HKEY_CURRENT_USER,
"SOFTWARE\\Microsoft\\Windows\\Shell\\Associations\\UrlAssociations\\https\\UserChoice",
"ProgId"
);
if (userChoice == null || userChoice.trim().isEmpty()) {
return ExtWebBrowser.IEXPLORE;
} else {
userChoice = userChoice.toUpperCase(Locale.ROOT);
}
```
##########
ide/extbrowser/src/org/netbeans/modules/extbrowser/NbDdeBrowserImpl.java:
##########
@@ -212,7 +271,15 @@ private String realDDEServer () {
}
try {
- String cmd = getDefaultOpenCommand ();
+ String cmd = getDefaultOpenCommand();
+
+ /** if not found with getDefaultWindowsOpenCommand function
+ * fallback to previous method
+ */
+ if (cmd.isEmpty()) {
+ cmd = getDefaultOpenCommand();
+ }
+
Review Comment:
the same method is called twice, something is wrong here.
this is probably supposed to be:
```java
String cmd = getDefaultWindowsOpenCommand();
if (cmd == null || cmd.trim().isEmpty()) {
cmd = getDefaultOpenCommand();
}
```
if the fallback is still required needs to be discussed. My feeling is that
it does the same thing just via native code, if true, it should be removed in
my opinion since it would be just a redundant call giving the same results.
##########
ide/extbrowser/src/org/netbeans/modules/extbrowser/NbDdeBrowserImpl.java:
##########
@@ -127,6 +133,59 @@ public NbDdeBrowserImpl (ExtWebBrowser extBrowserFactory) {
*/
public static native String getDefaultOpenCommand() throws
NbBrowserException;
+ /**
+ * Get the default browser name using Java JNA library
+ * @return String
+ */
+ private static String getDefaultWindowsBrowser() {
+ String userChoice = Advapi32Util
+ .registryGetStringValue(
+ WinReg.HKEY_CURRENT_USER,
+
"SOFTWARE\\Microsoft\\Windows\\Shell\\Associations\\UrlAssociations\\https\\UserChoice",
+ "ProgId"
+ )
+ .toUpperCase(Locale.ROOT);
+
+ // done this way so that values like FirefoxURL-308046B0AF4A39CB can
be handled
+ if (userChoice.toUpperCase().contains(ExtWebBrowser.FIREFOX)) {
+ return ExtWebBrowser.FIREFOX;
+ }
+ else if (userChoice.toUpperCase().contains(ExtWebBrowser.CHROME)) {
+ return ExtWebBrowser.CHROME;
+ } else if (userChoice.toUpperCase().contains(ExtWebBrowser.CHROMIUM)) {
+ return ExtWebBrowser.CHROMIUM;
+ } else if (userChoice.toUpperCase().contains(ExtWebBrowser.MOZILLA)) {
+ return ExtWebBrowser.MOZILLA;
+ } else {
+ return ExtWebBrowser.IEXPLORE;
+ }
Review Comment:
remove all `.toUpperCase()` from here since it is already in upper case at
this point.
##########
ide/extbrowser/src/org/netbeans/modules/extbrowser/NbDdeBrowserImpl.java:
##########
@@ -127,6 +133,59 @@ public NbDdeBrowserImpl (ExtWebBrowser extBrowserFactory) {
*/
public static native String getDefaultOpenCommand() throws
NbBrowserException;
+ /**
+ * Get the default browser name using Java JNA library
+ * @return String
+ */
+ private static String getDefaultWindowsBrowser() {
+ String userChoice = Advapi32Util
+ .registryGetStringValue(
+ WinReg.HKEY_CURRENT_USER,
+
"SOFTWARE\\Microsoft\\Windows\\Shell\\Associations\\UrlAssociations\\https\\UserChoice",
+ "ProgId"
+ )
+ .toUpperCase(Locale.ROOT);
+
+ // done this way so that values like FirefoxURL-308046B0AF4A39CB can
be handled
+ if (userChoice.toUpperCase().contains(ExtWebBrowser.FIREFOX)) {
+ return ExtWebBrowser.FIREFOX;
+ }
+ else if (userChoice.toUpperCase().contains(ExtWebBrowser.CHROME)) {
+ return ExtWebBrowser.CHROME;
+ } else if (userChoice.toUpperCase().contains(ExtWebBrowser.CHROMIUM)) {
+ return ExtWebBrowser.CHROMIUM;
+ } else if (userChoice.toUpperCase().contains(ExtWebBrowser.MOZILLA)) {
+ return ExtWebBrowser.MOZILLA;
+ } else {
+ return ExtWebBrowser.IEXPLORE;
+ }
+ }
+
+ /**
+ * Retrieves the browser execution path from the registry using the java
JNA library
+ * @param browser
+ * @return String
+ */
+ private static String getDefaultWindowsOpenCommandPath(String browser) {
+ String executionCommand = Advapi32Util
+ .registryGetStringValue(
+ WinReg.HKEY_CLASSES_ROOT,
+ "Applications\\" + browser.toLowerCase() +
".exe\\shell\\open\\command",
+ ""
+ );
+
+ // ensures a null value is never returned
+ if (executionCommand == null) {
+ return new String();
+ } else {
+ return executionCommand;
+ }
Review Comment:
its ok to return null here since empty string isn't useful either
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
