User-Agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:126.96.36.199) Gecko/20060201 Firefox/188.8.131.52 (Ubuntu-dapper) Build Identifier: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:184.108.40.206) Gecko/20060201 Firefox/220.127.116.11 (Ubuntu-dapper) http://lxr.mozilla.org/seamonkey/source/toolkit/components/remote/nsGTKRemoteService.cpp#370 FindChar should be RFind Reproducible: Always Steps to Reproduce: 1. Start firefox. 2. Type $ firefox -remote 'openURL(http://www.google.com/search?q=)+google)' Actual Results: $ firefox -remote 'openURL(http://www.google.com/search?q=)+google)' Error: Failed to send command: 500 command not parseable Expected Results: A new tab or window should open with the given URL. We can avoid this error by replacing ")" by "%29" I found the source of this bug in this line: http://lxr.mozilla.org/seamonkey/source/toolkit/components/remote/nsGTKRemoteService.cpp#370 FindChar should be RFindChar, as in the correct parsing in: http://lxr.mozilla.org/seamonkey/source/xpfe/components/xremote/src/XRemoteService.cpp#94
Summary: firefox -remote 'openURL(<URL>)' doesn't work when <URL> a closing parenthesis ")" → firefox -remote 'openURL(<URL>)' doesn't work when <URL> contains a closing parenthesis ")"
Confirming and reassigning. The cause of the problem is likely here: http://lxr.mozilla.org/mozilla/source/browser/components/nsBrowserContentHandler.js#334 However, since the x-remote spec doesn't mention any way to quote special characters like ")", this may end up as WONTFIX. Gavin: Regarding your statement in bug 298960 comment #42, do you think that we want to support this use case? Being able to quote the URL would certainly help here.
Status: UNCONFIRMED → NEW
Component: General → Startup and Profile System
Ever confirmed: true
QA Contact: general → startup
(In reply to comment #1) > Gavin: > Regarding your statement in bug 298960 comment #42, do you think that we want > to support this use case? Being able to quote the URL would certainly help > here. I don't have a strong opinion. If someone steps up to do the work and ensure that we don't regress any of the current functionality I doubt that we'd reject the patch :)
Since this bug is about ")" in particular and not quoting in general (which might be overkill), let's try a minimal fix here. This patch changes the regexp in nsBrowserContentHandler.js to allow a closing parenthesis within the URL. The remote command from comment #0 works with this patch.
Assignee: nobody → elmar.ludwig
Status: NEW → ASSIGNED
Attachment #292816 - Flags: review?(gavin.sharp)
Comment on attachment 292816 [details] [diff] [review] minimal patch v1 Switching review request to Benjamin Smedberg.
Attachment #292816 - Flags: review?(gavin.sharp) → review?(benjamin)
The only reason I haven't reviewed this is because I haven't had the chance to test it properly. It'd be nice to at least get confirmation that you've tested it with the command lines from from here and bug 298960 comment 34. An automated test that covers those cases would be ideal - you might be able to do something hacky like use the browser-chrome test harness and subscript-load nsBrowserContentHandler, and then call handle() directly with a fake nsICommandLine object.
If you prefer to still review this, feel free to do so. I just though you didn't have the time. Regarding the tests: I have verified that the command line from this bug (and some variations like opening in a new tab) as well as the ones listed in bug 298960 comment 34 work with this patch, although I'm not sure I understand your remark about the last case in that comment. Alas, my JS hacking skills are probably not good enough to adapt the existing browser-chrome test harness to create an automated test for this. I could try, but that might take a while...
Comment on attachment 292816 [details] [diff] [review] minimal patch v1 I decline to review this without a unit test of some sort... probably an xpcshell one would be sufficient.
Attachment #292816 - Flags: review?(benjamin)
I won't have any time for this, so moving to default owner.
Assignee: elmar.ludwig → nobody
Status: ASSIGNED → NEW
Status: NEW → RESOLVED
Closed: 3 months ago
Depends on: 1080319
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.