Add JNA code to retrieve default browser from Windows registry #6626
Conversation
mbien
added
the
ci:dev-build
| String userChoice = Advapi32Util | ||
| .registryGetStringValue( | ||
| WinReg.HKEY_CURRENT_USER, | ||
| "SOFTWARE\\Microsoft\\Windows\\Shell\\Associations\\UrlAssociations\\https\\UserChoice", | ||
| "ProgId" | ||
| ) | ||
| .toUpperCase(Locale.ROOT); |
There was a problem hiding this comment.
this will throw a NPE if registryGetStringValue(...) returns null
change to:
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);
}| 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; | ||
| } |
There was a problem hiding this comment.
remove all .toUpperCase() from here since it is already in upper case at this point.
There was a problem hiding this comment.
When I backed up my fork before deleting and reforking netbeans I've somehow managed to grab the previous code configuration for this function 😫
Now fixed again
| // ensures a null value is never returned | ||
| if (executionCommand == null) { | ||
| return new String(); | ||
| } else { | ||
| return executionCommand; | ||
| } |
There was a problem hiding this comment.
its ok to return null here since empty string isn't useful either
There was a problem hiding this comment.
I was hoping to avoid causing a null related error, I have changed it back to how it was before
| String cmd = getDefaultOpenCommand (); | ||
| String cmd = getDefaultOpenCommand(); | ||
|
|
||
| /** if not found with getDefaultWindowsOpenCommand function | ||
| * fallback to previous method | ||
| */ | ||
| if (cmd.isEmpty()) { | ||
| cmd = getDefaultOpenCommand(); | ||
| } | ||
|
|
There was a problem hiding this comment.
the same method is called twice, something is wrong here.
this is probably supposed to be:
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.
There was a problem hiding this comment.
I think I was in to much of a rush to resubmit the pull request that I managed to missed this
The previous method tries to work out the default browser by looking in HKEY_CLASSES_ROOT at .html, the new method finds it by looking in HKEY_CURRENT_USER at the userChoice for https.
dragonsKnight5
force-pushed
the
master
branch
from
639f41a
to
33a9cb8
Compare
|
I believe I have corrected all of my id10t errors. Microsoft have depreciated internet explorer, do I need to add any thing to deal with this? Here's the depreciation announcement link |
| } else if (userChoice).contains(ExtWebBrowser.MOZILLA)) { | ||
| return ExtWebBrowser.MOZILLA; |
There was a problem hiding this comment.
userChoice). should be userChoice.
| "SOFTWARE\\Microsoft\\Windows\\Shell\\Associations\\UrlAssociations\\https\\UserChoice", | ||
| "ProgId" | ||
| ) | ||
| .toUpperCase(Locale.ROOT); |
There was a problem hiding this comment.
need import java.util.Locale
| else if (userChoice.contains(ExtWebBrowser.FIREFOX)) { | ||
| return ExtWebBrowser.FIREFOX; | ||
| } | ||
| else if (userChoicecontains(ExtWebBrowser.CHROME)) { |
dragonsKnight5
force-pushed
the
master
branch
from
33a9cb8
to
bfc2731
Compare
|
I have now corrected these syntax errors, I feel like a bit of an idiot for missing these 😬 |
| @@ -19,6 +19,10 @@ | |||
|
|
|||
| package org.netbeans.modules.extbrowser; | |||
|
|
|||
| import com.sun.jna.platform.win32.Advapi32Util; | |||
| import com.sun.jna.platform.win32.WinReg; | |||
| import java.util.Locale | |||
There was a problem hiding this comment.
missing ; at end of line. You should try to compile using ant to get rid of the "basic compiler issue"
| else if (user.Choicecontains(ExtWebBrowser.CHROME)) { | ||
| return ExtWebBrowser.CHROME; |
There was a problem hiding this comment.
still missplaced . should be userChoice.contains
|
please make sure that it builds and works as expected locally first, before requesting reviews. Reviews are not there to fix compiler errors which can be easily found locally. |
update testGetDefaultOpenCommand to use new JNA based method update testGetDefaultOpenCommand to use new JNA based method update testGetDefaultOpenCommand to use new JNA based method Add JNA code to retrieve default browser from Windows registry add JNA dependency Add JNA code to retrieve default browser from Windows registry correct ID10t errors made by me Add JNA code to retrieve default browser from Windows registry fix syntax errors fix errors Add JNA code to retrieve default browser from Windows registry
dragonsKnight5
force-pushed
the
master
branch
from
bfc2731
to
8bcb2b0
Compare
|
I honestly thought I had rectified all errors, it's not my intention to make a nuisance of myself I've put the build log below as proof that it compiles and runs
|
changes the default browser selection behaviour when executed on Windows to use the users set default browser
related to issue "Default browser on Windows #3960"