Community
Participate
Working Groups
Didn't find the 'org.eclipse.ui.browser' project so I reported the bug here. Creating an external browser instance with parameters (ex: -incognito ) and then using that external browser instance to open a certain url converts the url to loercase . Url's are case sensitive so not all url's work. The problem is because of some changes last year to java Runtime.exec() which should accept an array of a command and arguments (each argument in it's own array cell) . In the code it concatenates the parameters and url's so when cmd reaches: process = Runtime.getRuntime().exec(cmd); It contains an arrray containing two cells [browser_executable, -incognito +URL ] It should be 3.
Please include a sample project or code snippet to show how this is being called. PW
An example on Windows 8.1, Internet Explorer 11, Eclipse 4.4M5, Java 1.7.0_51 Preferences->General->Web Browser Select "Use external web browser" Choose "Internet Explorer" Edit Parameters -> "-private" Open any html file by Open With->Web Browser Result: Internet Explorer opens Bing with "-private file:/C: ... .html" as searching text Expected: Internet Explorer opens file:/C: ... .html in private mode
I suspect the problem is in : org.eclipse.ui.internal.browser.ExternalBrowserInstance.openURL(URL) more specifficaly this line:
(Continued from previews comment , hit shift enter and posted the comment) The problem is this line : String params = WebBrowserUtil.createParameterString(parameters, urlText); It concatenates the -private (-incognito) with the url parameter . they should be in separate array cells for Runtime.exec() new behavior to work
(In reply to Toshihiro Izumi from comment #2) > An example on Windows 8.1, Internet Explorer 11, Eclipse 4.4M5, Java 1.7.0_51 > > Preferences->General->Web Browser > Select "Use external web browser" > Choose "Internet Explorer" > Edit Parameters -> "-private" > > Open any html file by Open With->Web Browser OK, thanks. As Ovidiu mentioned, the problem is probably in org.eclipse.ui.internal.browser.ExternalBrowserInstance.openURL(URL). I can't reproduce on linux, as org.eclipse.ui.internal.browser.browsers.MozillaBrowser.BrowserThread.openBrowser(String, String) tokenizes the string before exec'ing. See https://wiki.eclipse.org/Platform_UI/How_to_Contribute PW
FWIW the Mac version of this uses 'open' with -a as the application id. Any arguments are currently dropped, but adding --args to the end of the open command will pass arguments through, e.g. open -a /Applications/Google\ Chrome.app/Contents/MacOS/Google\ Chrome --args --kiosk
Created attachment 252194 [details] mylyn/context/zip
Opened bug 463949 for Alex's point
Gerrit change https://git.eclipse.org/r/45270 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=cdb1728544d0cd7429ccee12cdd23bf74a037130
Thanks Tomasz! Fixed for 4.5 M7
(In reply to Eclipse Genie from comment #9) > Gerrit change https://git.eclipse.org/r/45270 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=cdb1728544d0cd7429ccee12cdd23bf74a037130 This commit caused build failure in the nightly build, see http://download.eclipse.org/eclipse/downloads/drops4/N20150407-2000/compilelogs/plugins/org.eclipse.ua.tests_3.3.800.qualifier/@dot.html Looks like /browser/org/eclipse/ua/tests/browser/external/TestParameterSubstitution.java still has reference to the old method.
So ... since caused a build failure ... either the change has to be reverted, or the test code that uses it fixed. But, makes me wonder if this is a "change in API"? I've only skimmed the comments, but when ever possible, we would want to leave the old version of a method around, marked it @deprecated perhaps even document how it "does not work right" ... but ... to "break" an API at this late point is not good. (And, even, if "internal" non-API, it is not good to remove public methods this late, unless a substantial reason to.) Make sense?
New Gerrit change created: https://git.eclipse.org/r/45431
Gerrit change https://git.eclipse.org/r/45431 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ua.git/commit/?id=301ecd46d78e8d0a13f2161ba93d6812edb94c9e
(In reply to David Williams from comment #12) > So ... since caused a build failure ... either the change has to be > reverted, or the test code that uses it fixed. Test fixed (I hope) > But, makes me wonder if this is a "change in API"? The package name contains internal so I think we should be fine here, IMHO this is not an API change.
String#replaceAll(..) uses regex replacement and would need proper escaping. Please revert that code to use substring(..) and string concatenation. The code from comment 9 leads to exceptions like this: Exception in thread "Thread-4" java.lang.IndexOutOfBoundsException: No group 1 at java.util.regex.Matcher.start(Matcher.java:375) at java.util.regex.Matcher.appendReplacement(Matcher.java:880) at java.util.regex.Matcher.replaceAll(Matcher.java:955) at java.lang.String.replaceAll(String.java:2210) at org.eclipse.ui.internal.browser.WebBrowserUtil.createParameterArray(WebBrowserUtil.java:369) at org.eclipse.ui.internal.browser.browsers.MozillaBrowser$BrowserThread.run(MozillaBrowser.java:172) Steps: - Prefs > Web Browser: Select Firefox as external web browser and set parameters to "-url %URL%" - restart Eclipse (bug 464153) - try to open a file called about$1.html
.
Reverted from String.replaceAll(). Sorry for the associated breakage.
(In reply to Lars Vogel from comment #15) > Test fixed (I hope) Thanks Lars. (In reply to Brian de Alwis from comment #18) > Reverted from String.replaceAll(). Thanks Brian.
(In reply to Lars Vogel from comment #15) > The package name contains internal so I think we should be fine here, IMHO > this is not an API change. I am not saying you are wrong (and you probably are not) but besides our "API rules" which we are strict about "not breaking", there are some methods, in "internal" packages that are in wide spread use that have sort of become defacto APIs ... and IMHO, we should be "friendly" and not break those either unless there is a substantial reason to. I know it's hard to know for sure which are in wide spread use, and which are not, but, just wanted to remind you of our friendly nature. :) In this case, was there an API that "does the same thing" and the Help system tests just wasn't using it? Or, is there no official API for "open URL"? I know WTP is a project that opens lots of URLs and I don't know what methods they use, or if they use APIs or not. So, I've added Chuck and Carl, in case they can arrange for someone to take an for an "early look". Just my 2 cents. Again, not saying you wrong, just encouraging everyone to use care in changing public methods.
New Gerrit change created: https://git.eclipse.org/r/45494
Gerrit change https://git.eclipse.org/r/45494 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=4ece6c7a8c4cd32420296741dd81158c77c19306
Tomasz restored the old method.
Looks like my adjustment of the tests is not correct, it causes test failures. Tomasz, can you have a look at decided if we should revert the test of the now deprecated method or if you want to adjust the tests?
New Gerrit change created: https://git.eclipse.org/r/45640
The fix for the UA Browser Tests wasn't correct as the return value of #createParameterArray() is different from #createParameterString() (array vs string). Since we've restored #createParameterArray() I've reverted the test change back to what was there previously.
Pushed up a better fix that tests both the #createParameterString() and #createParameterArray() variants. We should move these tests to org.eclipse.ui.tests at some point?
Gerrit change https://git.eclipse.org/r/45640 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ua.git/commit/?id=1f843a17e5e8371aebeb724f0c1541a4208da630
Marking as fixed. Hoping for the last time!
(In reply to Brian de Alwis from comment #27) > Pushed up a better fix that tests both the #createParameterString() and > #createParameterArray() variants. We should move these tests to > org.eclipse.ui.tests at some point? +1, the move makes sense to avoid future cross dependencies. Thanks for the fix.
(In reply to Lars Vogel from comment #30) > +1, the move makes sense to avoid future cross dependencies. See bug 464474.
Verified in I20150428-0800.
