New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Firefox 38.8.0 Beta 7 command line parsing error #176

Closed
RonKlein32 opened this Issue May 30, 2016 · 8 comments

Comments

Projects
None yet
5 participants
@RonKlein32

RonKlein32 commented May 30, 2016

Executing the following command from an OS/2 command line results in File not Found error:
e:\firefox\firefox.exe file:///g:\tmp\mail.htm
This command has worked in all previous versions of netscape and firefox that I have used. The address line in firefox reads 'file:///g://tmp//mail.htm' which is obviously incorrect.
Ron Klein

@dmik

This comment has been minimized.

Show comment
Hide comment
@dmik

dmik May 30, 2016

Contributor

I have no idea how you get this result. Here running file:///D:\Tests\timerTest.html from CMD.EXE works perfectly well (the address becomes file:///D:/Tests/timerTest.html), no matter which version of Firefox I use (including the one you mention). Please provide more details on how exactly you start Firefox.

Contributor

dmik commented May 30, 2016

I have no idea how you get this result. Here running file:///D:\Tests\timerTest.html from CMD.EXE works perfectly well (the address becomes file:///D:/Tests/timerTest.html), no matter which version of Firefox I use (including the one you mention). Please provide more details on how exactly you start Firefox.

@RonKlein32

This comment has been minimized.

Show comment
Hide comment
@RonKlein32

RonKlein32 May 30, 2016

Firefox is already running. This command opens a new tab in the
existing Firefox. Firefox is started from an OS/2 program object with
no parameters. No other Mozilla app is running.

I did try executing the command directly from the firefox directory
(i.e. firefox.exe without the path) and it works correctly.

Ron Klein

In bitwiseworks/mozilla-os2/issues/176/222537358@github.com, on
05/30/16
at 11:20 AM, Dmitriy Kuminov notifications@github.com said:

I have no idea how you get this result. Here running
file:///D:\Tests\timerTest.html from CMD.EXE works perfectly well
(the address becomes file:///D:/Tests/timerTest.html), no matter
which version of Firefox I use (including the one you mention). Please
provide more details on how exactly you start Firefox.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#176 (comment)
-<Attachment #1, unnamed (1291 bytes), text/html 7bit>-


rklein46@roadrunner.com
MR/2 Internet Cruiser Edition for OS/2 v2.67

eComStation 2.2 Beta

RonKlein32 commented May 30, 2016

Firefox is already running. This command opens a new tab in the
existing Firefox. Firefox is started from an OS/2 program object with
no parameters. No other Mozilla app is running.

I did try executing the command directly from the firefox directory
(i.e. firefox.exe without the path) and it works correctly.

Ron Klein

In bitwiseworks/mozilla-os2/issues/176/222537358@github.com, on
05/30/16
at 11:20 AM, Dmitriy Kuminov notifications@github.com said:

I have no idea how you get this result. Here running
file:///D:\Tests\timerTest.html from CMD.EXE works perfectly well
(the address becomes file:///D:/Tests/timerTest.html), no matter
which version of Firefox I use (including the one you mention). Please
provide more details on how exactly you start Firefox.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#176 (comment)
-<Attachment #1, unnamed (1291 bytes), text/html 7bit>-


rklein46@roadrunner.com
MR/2 Internet Cruiser Edition for OS/2 v2.67

eComStation 2.2 Beta

@dmik

This comment has been minimized.

Show comment
Hide comment
@dmik

dmik May 30, 2016

Contributor

Yes, this is a regression of FF 38 (i.e. it kinda works in FF31 and earlier except that in FF31 it opens a new window instead of a new tab which was fixed for 38.8.0). I will look into it before the next release. For now, as a workaround, please use forward slashes instead of backward ones — this works well.

Contributor

dmik commented May 30, 2016

Yes, this is a regression of FF 38 (i.e. it kinda works in FF31 and earlier except that in FF31 it opens a new window instead of a new tab which was fixed for 38.8.0). I will look into it before the next release. For now, as a workaround, please use forward slashes instead of backward ones — this works well.

@dmik dmik added this to the Firefox Beta 8 milestone May 30, 2016

@dmik dmik added the bug label May 30, 2016

@dazarewicz

This comment has been minimized.

Show comment
Hide comment
@dazarewicz

dazarewicz Jun 6, 2016

This problem also exists when opening a URL from PMMail (and likely other applications), something I need to do repeatedly every day. There is no option to control the formation of the file name in that case (ie. create a non-standard OS/2 file name using forward slashes). It just doesn't work anymore. However you can manually edit the file name in the URL entry field.

dazarewicz commented Jun 6, 2016

This problem also exists when opening a URL from PMMail (and likely other applications), something I need to do repeatedly every day. There is no option to control the formation of the file name in that case (ie. create a non-standard OS/2 file name using forward slashes). It just doesn't work anymore. However you can manually edit the file name in the URL entry field.

@dspiatkowski

This comment has been minimized.

Show comment
Hide comment
@dspiatkowski

dspiatkowski Jan 10, 2017

I just encountered this issue with my PMMail/FF installation. Originally posted on OS2Forum (see => http://www.os2world.com/forum/index.php/topic,1259.0.html.

As best as I can tell it is directly related to the 1st forward-slash following the drive letter, in my case:

  1. FF attempts to load the following link:

file:///G://APPS//TCPIP//PMMAIL//PMMAIL//TEMP//TEMP.HTM

where as if I invoke this directly in FF (where it works successfully):

file:///G:/APPS//TCPIP/PMMAIL/PMMAIL/TEMP/TEMP.HTM

So working through this I find that the only difference is the '//' vs '/' right after the 'G:' drive letter. The remaining directory separators can be either a single or double forward-slash (at least here in my testing either one works fine).

dspiatkowski commented Jan 10, 2017

I just encountered this issue with my PMMail/FF installation. Originally posted on OS2Forum (see => http://www.os2world.com/forum/index.php/topic,1259.0.html.

As best as I can tell it is directly related to the 1st forward-slash following the drive letter, in my case:

  1. FF attempts to load the following link:

file:///G://APPS//TCPIP//PMMAIL//PMMAIL//TEMP//TEMP.HTM

where as if I invoke this directly in FF (where it works successfully):

file:///G:/APPS//TCPIP/PMMAIL/PMMAIL/TEMP/TEMP.HTM

So working through this I find that the only difference is the '//' vs '/' right after the 'G:' drive letter. The remaining directory separators can be either a single or double forward-slash (at least here in my testing either one works fine).

@dmik

This comment has been minimized.

Show comment
Hide comment
@dmik

dmik Jun 19, 2017

Contributor

Somehow I can't reproduce this any more, neither in FF 38.8.0 nor in FF 45.9.0. Yes, if you pass backward slashes on the command line, then they appear as // in the URL bar but the file loads fine. I guess it's because multiple forward slashes should be ignored by the standard. In Safari/Chrome/FF for Mac they are ignored (interpreted as there is just one slash).

Contributor

dmik commented Jun 19, 2017

Somehow I can't reproduce this any more, neither in FF 38.8.0 nor in FF 45.9.0. Yes, if you pass backward slashes on the command line, then they appear as // in the URL bar but the file loads fine. I guess it's because multiple forward slashes should be ignored by the standard. In Safari/Chrome/FF for Mac they are ignored (interpreted as there is just one slash).

@dmik

This comment has been minimized.

Show comment
Hide comment
@dmik

dmik Jun 19, 2017

Contributor

Why FF gets double slashes is another story. I guess that backward slashes are converted to \\ (escaped) before passing to a running FF instance via DDE and then are just blindly converted to forward slashes. Note that if you run a new instance instead of opening a new tab in the existing instance, there are single forward slashes. So it's definitely DDE. I remember there is some code that did slash escaping.

Contributor

dmik commented Jun 19, 2017

Why FF gets double slashes is another story. I guess that backward slashes are converted to \\ (escaped) before passing to a running FF instance via DDE and then are just blindly converted to forward slashes. Note that if you run a new instance instead of opening a new tab in the existing instance, there are single forward slashes. So it's definitely DDE. I remember there is some code that did slash escaping.

@dmik

This comment has been minimized.

Show comment
Hide comment
@dmik

dmik Jun 20, 2017

Contributor

I found why we started getting double \\ in FF 38 and later. It's a regression of my fix of the broken command line processing for FF 38 beta 7 in #168. The thing is that Firefox expects a flattened command line argument string in several places (and opening a new tab in an already running FF instance is one of them).

The code responsible for parsing such a string on OS/2 is nsNativeAppSupportOS2::HandleCommandLine. And this is modeled exactly after the respective Windows code (nsNativeAppSupportWin::HandleCommandLine). And the Windows code in turn closely follows the MS spec called "Parsing C++ Command-Line Arguments" (see https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments). This spec only requires to escape backslashes with backslashes if they immediately precede a quote (this requirement sounds really strange to me). While my fix from #168 escapes all of them now (like on Unix). Hence double backslashes.

Given that this only works by accident (because the URL spec requires to ignore double slashes), this needs to be fixed as the command line may be used not only for URLs in Firefox. The responsive code is SendRequest in nsNativeAppSupportOS2.cpp. The fix is trivial but requires some careful work to fully match the strange MS spec.

Contributor

dmik commented Jun 20, 2017

I found why we started getting double \\ in FF 38 and later. It's a regression of my fix of the broken command line processing for FF 38 beta 7 in #168. The thing is that Firefox expects a flattened command line argument string in several places (and opening a new tab in an already running FF instance is one of them).

The code responsible for parsing such a string on OS/2 is nsNativeAppSupportOS2::HandleCommandLine. And this is modeled exactly after the respective Windows code (nsNativeAppSupportWin::HandleCommandLine). And the Windows code in turn closely follows the MS spec called "Parsing C++ Command-Line Arguments" (see https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments). This spec only requires to escape backslashes with backslashes if they immediately precede a quote (this requirement sounds really strange to me). While my fix from #168 escapes all of them now (like on Unix). Hence double backslashes.

Given that this only works by accident (because the URL spec requires to ignore double slashes), this needs to be fixed as the command line may be used not only for URLs in Firefox. The responsive code is SendRequest in nsNativeAppSupportOS2.cpp. The fix is trivial but requires some careful work to fully match the strange MS spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment