View Issue Details

IDProjectCategoryView StatusLast Update
0021651LazarusPatchpublic2012-05-15 12:37
ReporterBigChimpAssigned ToFelipe Monteiro de Carvalho 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformMac Intel+PC x64OSOSX/WindowsOS VersionLion/Vista
Product Version0.9.31 (SVN)Product Build35900/36173 
Target VersionFixed in Version1.1 (SVN) 
Summary0021651: [Patch] OpenDocument requires " around file on OSX, not on Windows
DescriptionIf you run lclintf.Opendocument with file names with spaces in the paths, you will need to double quote them on OSX. Not quoting them won't work and apparently won't even return failure.
On Windows, you don't need to quote the file/path.
Code inspection of lcl/include/sysenvapis_unix.inc suggests you don't need to quote on other Unix either.
Steps To ReproduceRun attached program.
Navigate to a pdf file with e.g. spaces in the path.
Click OpenDocument.
FileExistsUTF8 will properly recognize the file is there.
The call to OpenDocument will do nothing though (apparently not even return a failure)
Additional InformationInconsistent behaviour:
- Windows version does not require quoting
- Inconsistent with other functions like FileExistsUTF8 that also don't require quoting (and won't work with it)

Compared recent Lazarus Windows x86 SVN with OSX version.

Attached (untested) patch will presumably fix it, but break earlier code.
Note that the Unix version also quotes the path for you, so OSX is the only one that is different:
  if (APath<>'') and (APath[1]<>'"') then
    APath:=QuotedStr(APath);
... perhaps using QuotedStr instead of the patch will also work

Below is a grep -i --recursive OpenDocument * from the Lazarus root directory; at first glance, seems changing this won't have consequences for Lazarus (only perhaps improvements in the Delphi converter on OSX):
Binary file components/lazreport/doc/.svn/text-base/firststeps.odt.svn-base matches
Binary file components/lazreport/doc/firststeps.odt matches
Binary file components/lazreport/doc/tutorials/stringgrid/en/.svn/text-base/Tutorial_en.odt.svn-base matches
Binary file components/lazreport/doc/tutorials/stringgrid/en/Tutorial_en.odt matches
Binary file components/lazreport/doc/tutorials/stringgrid/es/.svn/text-base/Tutorial_es.odt.svn-base matches
Binary file components/lazreport/doc/tutorials/stringgrid/es/Tutorial_es.odt matches
converter/.svn/text-base/convertsettings.pas.svn-base: 'if $3 match ":/" then OpenURL($3); OpenDocument($3)', 'LCL', 'LCLIntf');
converter/.svn/text-base/replacefuncsunit.pas.svn-base:// Example: 'if $3 match ":/" then OpenURL($3); OpenDocument($3)'
converter/convertsettings.pas: 'if $3 match ":/" then OpenURL($3); OpenDocument($3)', 'LCL', 'LCLIntf');
converter/replacefuncsunit.pas:// Example: 'if $3 match ":/" then OpenURL($3); OpenDocument($3)'
Binary file docs/booth/.svn/text-base/ProdProgEntwMitOpenSourceSystems2007.odp.svn-base matches
Binary file docs/booth/ProdProgEntwMitOpenSourceSystems2007.odp matches
Binary file docs/diagrams/.svn/text-base/customdrawn_diagram.odg.svn-base matches
Binary file docs/diagrams/customdrawn_diagram.odg matches
docs/xml/lcl/.svn/text-base/lclintf.xml.svn-base: <element name="OpenDocument">
docs/xml/lcl/.svn/text-base/lclintf.xml.svn-base: <element name="OpenDocument.Result">
docs/xml/lcl/.svn/text-base/lclintf.xml.svn-base: <element name="OpenDocument.APath">
docs/xml/lcl/lclintf.xml: <element name="OpenDocument">
docs/xml/lcl/lclintf.xml: <element name="OpenDocument.Result">
docs/xml/lcl/lclintf.xml: <element name="OpenDocument.APath">
Binary file lazarus.exe matches
Binary file lazarus.new.exe matches
Binary file lazarus.old.exe matches
lcl/.svn/text-base/lclintf.pas.svn-base:function OpenDocument(APath: String): Boolean;
lcl/include/.svn/text-base/sysenvapis_mac.inc.svn-base:function OpenDocument(APath: String): Boolean;
lcl/include/.svn/text-base/sysenvapis_unix.inc.svn-base:function OpenDocument(APath: String): Boolean;
lcl/include/.svn/text-base/sysenvapis_win.inc.svn-base:function OpenDocument(APath: String): Boolean;
lcl/include/sysenvapis_mac.inc:function OpenDocument(APath: String): Boolean;
lcl/include/sysenvapis_unix.inc:function OpenDocument(APath: String): Boolean;
lcl/include/sysenvapis_win.inc:function OpenDocument(APath: String): Boolean;
lcl/interfaces/carbon/.svn/text-base/carbonobject.inc.svn-base: AEInstallEventHandler(kCoreEventClass, kAEOpenDocuments, FOpenEventHandlerUPP, 0, False),
lcl/interfaces/carbon/carbonobject.inc: AEInstallEventHandler(kCoreEventClass, kAEOpenDocuments, FOpenEventHandlerUPP, 0, False),
lcl/interfaces/carbon/pascocoa/appkit/.svn/text-base/NSResponder.inc.svn-base:You can invoke this method to present error alert dialog boxes. For example, Cocoa's own [NSDocumentController openDocument:] invokes this method when it's just invoked -openDocumentWithContentsOfURL:display:error: and that method has returned nil.
lcl/interfaces/carbon/pascocoa/appkit/NSResponder.inc:You can invoke this method to present error alert dialog boxes. For example, Cocoa's own [NSDocumentController openDocument:] invokes this method when it's just invoked -openDocumentWithContentsOfURL:display:error: and that method has returned nil.
lcl/lclintf.pas:function OpenDocument(APath: String): Boolean;
Binary file lcl/units/i386-win32/lclintf.o matches
Binary file lcl/units/i386-win32/lclintf.ppu matches
Binary file lcl/units/x86_64-win64/lclintf.o matches
Binary file lcl/units/x86_64-win64/lclintf.ppu matches
Binary file tools/lazdatadesktop/lazdatadesktop.exe matches
Binary file units/i386-win32/win32/convertsettings.o matches

TagsNo tags attached.
Fixed in Revision37286
LazTarget-
WidgetsetWin32/Win64, Carbon
Attached Files
  • OpenDocumentOSX.diff (385 bytes)
    Index: lcl/include/sysenvapis_mac.inc
    ===================================================================
    --- lcl/include/sysenvapis_mac.inc	(revision 36574)
    +++ lcl/include/sysenvapis_mac.inc	(working copy)
    @@ -21,5 +21,5 @@
     function OpenDocument(APath: String): Boolean;
     begin
       Result := True;
    -  RunCmdFromPath('open',APath);
    +  RunCmdFromPath('open','"'+APath+'"');
     end;
    
    OpenDocumentOSX.diff (385 bytes)
  • PDFOpenTest.zip (4,109 bytes)
  • OpenDocumentOSX2.diff (709 bytes)
    Index: lcl/include/sysenvapis_mac.inc
    ===================================================================
    --- lcl/include/sysenvapis_mac.inc	(revision 36650)
    +++ lcl/include/sysenvapis_mac.inc	(working copy)
    @@ -19,7 +19,13 @@
     
     // Open a document with the default application associated with it in the system
     function OpenDocument(APath: String): Boolean;
    +var
    +  ResultingPath: string;
     begin
    -  Result := True;
    -  RunCmdFromPath('open',APath);
    +  Result := True;  
    +  if not FileExistsUTF8(APath) then exit(false);  
    +  // Paths with spaces need to be quoted:
    +  if (APath<>'') and (APath[1]<>'''') then
    +    ResultingPath:=QuotedStr(ResultingPath);  
    +  RunCmdFromPath('open',ResultingPath);
     end;
    
    OpenDocumentOSX2.diff (709 bytes)
  • Testprogram_Opendoc.zip (3,811 bytes)
  • OpenDocumentOSX3.diff (705 bytes)
    Index: lcl/include/sysenvapis_mac.inc
    ===================================================================
    --- lcl/include/sysenvapis_mac.inc	(revision 35900)
    +++ lcl/include/sysenvapis_mac.inc	(working copy)
    @@ -19,7 +19,15 @@
     
     // Open a document with the default application associated with it in the system
     function OpenDocument(APath: String): Boolean;
    +var
    +  ResultingPath: string;
     begin
       Result := True;
    -  RunCmdFromPath('open',APath);
    +  if not FileExistsUTF8(APath) then exit(false);
    +  // Paths with spaces need to be quoted:
    +  if (APath<>'') and (APath[1]<>'''') then
    +    ResultingPath:=QuotedStr(APath)
    +  else
    +    ResultingPath:=APath;
    +  RunCmdFromPath('open',ResultingPath);
     end;
    
    OpenDocumentOSX3.diff (705 bytes)

Relationships

related to 0021659 closedBart Broersma [Patch] OpenURL on Windows needs double quotes for file:// URL 

Activities

2012-04-05 15:14

 

OpenDocumentOSX.diff (385 bytes)
Index: lcl/include/sysenvapis_mac.inc
===================================================================
--- lcl/include/sysenvapis_mac.inc	(revision 36574)
+++ lcl/include/sysenvapis_mac.inc	(working copy)
@@ -21,5 +21,5 @@
 function OpenDocument(APath: String): Boolean;
 begin
   Result := True;
-  RunCmdFromPath('open',APath);
+  RunCmdFromPath('open','"'+APath+'"');
 end;
OpenDocumentOSX.diff (385 bytes)

Felipe Monteiro de Carvalho

2012-04-06 15:13

developer   ~0058357

Like from the other bug: I wonder if it should only add quotes when there are actually spaces. It would be safer.

Reinier Olislagers

2012-04-06 15:26

developer   ~0058361

Last edited: 2012-04-07 09:46

OSX man page suggests using single quotes:
http://developer.apple.com/library/mac/#documentation/Darwin/Reference/ManPages/man1/open.1.html
... but they indeed only show example with spaces. I'd prefer testing the patch as is with paths without spaces; these should still work.

... e.g. http://serverfault.com/questions/69283/when-to-use-single-quote-double-quote-in-grep confirms this: single quotes won't expand variable names etc.

An argument I specified above: the regular Unix version also always quotes the argument.... but in a way you're right: this patch doesn't check things the way the Unix patch does. New patch coming up.

Updated: are you saying it's safer to only quote when spaces are present in order not to break existing code? In that case, implementers will already have quoted it (single or double) otherwise the function won't work.
Checking for spaces won't work and only double up quotes unless existing quotes are detected. The new patch does that more or less like the Unix version.
It will still fail if double quotes are used though.

2012-04-07 09:03

 

PDFOpenTest.zip (4,109 bytes)

Reinier Olislagers

2012-04-07 09:03

developer   ~0058382

Last edited: 2012-04-07 09:49

Oops, forget test program, sorry. Uploaded it now.
Created new patch:
- rudimentary test for existing single quotes and refrains from quoting (adapted from Unix version).
- does not test for double quotes
- tests for document existence taken from Unix version - otherwise the function would always return true.

Going to test it now.

2012-04-07 09:47

 

OpenDocumentOSX2.diff (709 bytes)
Index: lcl/include/sysenvapis_mac.inc
===================================================================
--- lcl/include/sysenvapis_mac.inc	(revision 36650)
+++ lcl/include/sysenvapis_mac.inc	(working copy)
@@ -19,7 +19,13 @@
 
 // Open a document with the default application associated with it in the system
 function OpenDocument(APath: String): Boolean;
+var
+  ResultingPath: string;
 begin
-  Result := True;
-  RunCmdFromPath('open',APath);
+  Result := True;  
+  if not FileExistsUTF8(APath) then exit(false);  
+  // Paths with spaces need to be quoted:
+  if (APath<>'') and (APath[1]<>'''') then
+    ResultingPath:=QuotedStr(ResultingPath);  
+  RunCmdFromPath('open',ResultingPath);
 end;
OpenDocumentOSX2.diff (709 bytes)

Reinier Olislagers

2012-04-07 11:08

developer   ~0058386

New patch. Tested on OSX Lion with files/paths containing spaces and those not containing spaces.
Suggest this be applied (or changed to test if spaces in file name if you think that is better)

Updated test program (indicates it works for all documents not just pdf; fixes for TProcess test on OSX)

2012-04-07 11:08

 

Testprogram_Opendoc.zip (3,811 bytes)

2012-04-07 11:08

 

OpenDocumentOSX3.diff (705 bytes)
Index: lcl/include/sysenvapis_mac.inc
===================================================================
--- lcl/include/sysenvapis_mac.inc	(revision 35900)
+++ lcl/include/sysenvapis_mac.inc	(working copy)
@@ -19,7 +19,15 @@
 
 // Open a document with the default application associated with it in the system
 function OpenDocument(APath: String): Boolean;
+var
+  ResultingPath: string;
 begin
   Result := True;
-  RunCmdFromPath('open',APath);
+  if not FileExistsUTF8(APath) then exit(false);
+  // Paths with spaces need to be quoted:
+  if (APath<>'') and (APath[1]<>'''') then
+    ResultingPath:=QuotedStr(APath)
+  else
+    ResultingPath:=APath;
+  RunCmdFromPath('open',ResultingPath);
 end;
OpenDocumentOSX3.diff (705 bytes)

Felipe Monteiro de Carvalho

2012-05-15 09:52

developer   ~0059606

thanks, applied

Issue History

Date Modified Username Field Change
2012-04-05 15:14 Reinier Olislagers New Issue
2012-04-05 15:14 Reinier Olislagers File Added: OpenDocumentOSX.diff
2012-04-05 15:14 Reinier Olislagers Widgetset => Win32/Win64, Carbon
2012-04-06 15:12 Felipe Monteiro de Carvalho Status new => assigned
2012-04-06 15:12 Felipe Monteiro de Carvalho Assigned To => Felipe Monteiro de Carvalho
2012-04-06 15:13 Felipe Monteiro de Carvalho Note Added: 0058357
2012-04-06 15:13 Felipe Monteiro de Carvalho Relationship added related to 0021659
2012-04-06 15:26 Reinier Olislagers Note Added: 0058361
2012-04-07 09:03 Reinier Olislagers File Added: PDFOpenTest.zip
2012-04-07 09:04 Reinier Olislagers Note Added: 0058382
2012-04-07 09:31 Reinier Olislagers Note Edited: 0058361
2012-04-07 09:35 Reinier Olislagers Note Edited: 0058361
2012-04-07 09:46 Reinier Olislagers Note Edited: 0058361
2012-04-07 09:47 Reinier Olislagers Note Edited: 0058382
2012-04-07 09:47 Reinier Olislagers File Added: OpenDocumentOSX2.diff
2012-04-07 09:49 Reinier Olislagers Note Edited: 0058382
2012-04-07 11:08 Reinier Olislagers Note Added: 0058386
2012-04-07 11:08 Reinier Olislagers File Added: Testprogram_Opendoc.zip
2012-04-07 11:08 Reinier Olislagers File Added: OpenDocumentOSX3.diff
2012-05-15 09:52 Felipe Monteiro de Carvalho Fixed in Revision => 37286
2012-05-15 09:52 Felipe Monteiro de Carvalho LazTarget => -
2012-05-15 09:52 Felipe Monteiro de Carvalho Status assigned => resolved
2012-05-15 09:52 Felipe Monteiro de Carvalho Fixed in Version => 1.1 (SVN)
2012-05-15 09:52 Felipe Monteiro de Carvalho Resolution open => fixed
2012-05-15 09:52 Felipe Monteiro de Carvalho Note Added: 0059606
2012-05-15 12:37 Reinier Olislagers Status resolved => closed