Merge branch 'xrc-paths'

Allow loading XRC from wxXmlDocument and use it to add a test for
bitmap paths URI encoding in XRC.

See https://github.com/wxWidgets/wxWidgets/pull/2325

docs/doxygen/overviews/xrc_format.h

@@ -341,8 +341,8 @@ or translations are done.
 @subsection overview_xrcformat_type_bitmap Bitmap
 
 Bitmap properties contain specification of a single bitmap or icon. In the most
-basic form, their text value is simply a relative filename (or another
+basic form, their text value is simply a relative URL of the bitmap to use.
-wxFileSystem URL) of the bitmap to use. For example:
+For example:
 @code
 <object class="tool" name="wxID_NEW">
     <tooltip>New</tooltip>
@@ -350,7 +350,15 @@ wxFileSystem URL) of the bitmap to use. For example:
 </object>
 @endcode
 The value is interpreted as path relative to the location of XRC file where the
-reference occurs.
+reference occurs, but notice that it is still an URL and not just a filename,
+which means that the characters special in the URLs, such as @c '#' must be
+percent-encoded, e.g. here is the correct way to specify a bitmap with the path
+@c "images/#1/tool.png" in XRC:
+@code
+<object class="tool" name="first">
+    <bitmap>images/%231/tool.png</bitmap>
+</object>
+@endcode
 
 Bitmap file paths can include environment variables that are expanded if
 wxXRC_USE_ENVVARS was passed to the wxXmlResource constructor.

include/wx/xrc/xmlres.h

@@ -131,6 +131,13 @@ public:
     // Loads all XRC files from a directory.
     bool LoadAllFiles(const wxString& dirname);
 
+    // Loads resources from the given XML document, taking ownership of it.
+    //
+    // The name argument is only used to Unload() the document later here and
+    // doesn't need to be an existing filename at all (but should be unique if
+    // specified, otherwise it's just synthesized internally).
+    bool LoadDocument(wxXmlDocument* doc, const wxString& name = wxString());
+
     // Unload resource from the given XML file (wildcards not allowed)
     bool Unload(const wxString& filename);
 
@@ -320,6 +327,9 @@ protected:
     // wxXmlDocument (which will be owned by caller) on success or NULL.
     wxXmlDocument *DoLoadFile(const wxString& file);
 
+    // Load XRC from the given document and returns true on success.
+    bool DoLoadDocument(const wxXmlDocument& doc);
+
     // Scans the resources list for unloaded files and loads them. Also reloads
     // files that have been modified since last loading.
     bool UpdateResources();

interface/wx/xrc/xmlres.h

@@ -233,12 +233,58 @@ public:
     */
     bool Load(const wxString& filemask);
 
+    /**
+        Load resources from the XML document containing them.
+
+        This can be useful when XRC contents comes from some place other than a
+        file or, more generally, an URL, as it can still be read into a
+        wxMemoryInputStream and then wxXmlDocument can be created from this
+        stream and used with this function.
+
+        For example:
+        @code
+        const char* const xrc_data = ...; // Retrieve it from wherever.
+        wxMemoryInputStream mis(xrc_data, strlen(xrc_data));
+        wxScopedPtr<wxXmlDocument> xmlDoc(new wxXmlDocument(mis, "UTF-8"));
+        if ( !xmlDoc->IsOk() )
+        {
+            ... handle invalid XML here ...
+            return;
+        }
+
+        if ( !wxXmlResource::Get()->LoadDocument(xmlDoc.release()) )
+        {
+            ... handle invalid XRC here ...
+            return;
+        }
+
+        ... use the just loaded XRC as usual ...
+        @endcode
+
+        @param doc A valid, i.e. non-null, document pointer ownership of which
+            is passed to wxXmlResource, i.e. this pointer can't be used after
+            this function rteturns.
+        @param name The name argument is optional, but may be provided if you
+            plan to call Unload() later. It doesn't need to be an existing file
+            or even conform to the usual form of file names as it is not
+            interpreted in any way by wxXmlResource, but it should be unique
+            among the other documents and file names used if specified.
+        @return @true on success, @false if the document couldn't be loaded
+            (note that @a doc is still destroyed in this case to avoid memory
+            leaks).
+
+        @see Load(), LoadFile()
+
+        @since 3.1.6
+     */
+    bool LoadDocument(wxXmlDocument* doc, const wxString& name = wxString());
+
     /**
         Simpler form of Load() for loading a single XRC file.
 
         @since 2.9.0
 
-        @see Load(), LoadAllFiles()
+        @see Load(), LoadAllFiles(), LoadDocument()
     */
     bool LoadFile(const wxFileName& file);
 

src/common/filesys.cpp

@@ -478,16 +478,17 @@ wxFSFile* wxFileSystem::OpenFile(const wxString& location, int flags)
     m_LastName.clear();
 
     // try relative paths first :
-    if (meta != wxT(':'))
+    if (meta != wxT(':') && !m_Path.empty())
     {
+        const wxString fullloc = m_Path + loc;
         node = m_Handlers.GetFirst();
         while (node)
         {
             wxFileSystemHandler *h = (wxFileSystemHandler*) node -> GetData();
-            if (h->CanOpen(m_Path + loc))
+            if (h->CanOpen(fullloc))
             {
-                s = MakeLocal(h)->OpenFile(*this, m_Path + loc);
+                s = MakeLocal(h)->OpenFile(*this, fullloc);
-                if (s) { m_LastName = m_Path + loc; break; }
+                if (s) { m_LastName = fullloc; break; }
             }
             node = node->GetNext();
         }

src/xrc/xmlres.cpp

@@ -74,17 +74,43 @@ wxDateTime GetXRCFileModTime(const wxString& filename)
 // name.
 static void XRCID_Assign(const wxString& str_id, int value);
 
+// Flags indicating whether the XRC contents was loaded from file or URL, more
+// generally: this is usually true but is not set for the records created
+// directly from wxXmlDocument.
+namespace XRCWhence
+{
+
+enum
+{
+    From_URL = 0,
+    From_Doc = 1
+};
+
+} // namespace // XRCWhence
+
 class wxXmlResourceDataRecord
 {
 public:
     // Ctor takes ownership of the document pointer.
     wxXmlResourceDataRecord(const wxString& File_,
-                            wxXmlDocument *Doc_
+                            wxXmlDocument *Doc_,
+                            int flags = XRCWhence::From_URL
                            )
         : File(File_), Doc(Doc_)
     {
 #if wxUSE_DATETIME
-        Time = GetXRCFileModTime(File);
+        switch ( flags )
+        {
+            case XRCWhence::From_URL:
+                Time = GetXRCFileModTime(File);
+                break;
+
+            case XRCWhence::From_Doc:
+                // Leave Time invalid.
+                break;
+        }
+#else
+        wxUnusedVar(flags);
 #endif
     }
 
@@ -664,9 +690,14 @@ bool wxXmlResource::UpdateResources()
         if ( m_flags & wxXRC_NO_RELOADING )
             continue;
 
+        // And we don't do it for the records that were not loaded from a
+        // file/URI (or at least not directly) in the first place.
+        if ( !rec->Time.IsValid() )
+            continue;
+
         // Otherwise check its modification time if we can.
 #if wxUSE_DATETIME
-        const wxDateTime lastModTime = GetXRCFileModTime(rec->File);
+        wxDateTime lastModTime = GetXRCFileModTime(rec->File);
 
         if ( lastModTime.IsValid() && lastModTime <= rec->Time )
 #else // !wxUSE_DATETIME
@@ -749,7 +780,15 @@ wxXmlDocument *wxXmlResource::DoLoadFile(const wxString& filename)
         return NULL;
     }
 
-    wxXmlNode * const root = doc->GetRoot();
+    if (!DoLoadDocument(*doc))
+        return NULL;
+
+    return doc.release();
+}
+
+bool wxXmlResource::DoLoadDocument(const wxXmlDocument& doc)
+{
+    wxXmlNode * const root = doc.GetRoot();
     if (root->GetName() != wxT("resource"))
     {
         ReportError
@@ -757,7 +796,7 @@ wxXmlDocument *wxXmlResource::DoLoadFile(const wxString& filename)
             root,
             "invalid XRC resource, doesn't have root node <resource>"
         );
-        return NULL;
+        return false;
     }
 
     long version;
@@ -778,7 +817,34 @@ wxXmlDocument *wxXmlResource::DoLoadFile(const wxString& filename)
     PreprocessForIdRanges(root);
     wxIdRangeManager::Get()->FinaliseRanges(root);
 
-    return doc.release();
+    return true;
+}
+
+bool wxXmlResource::LoadDocument(wxXmlDocument* doc, const wxString& name)
+{
+    wxCHECK_MSG( doc, false, wxS("must have a valid document") );
+
+    if ( !DoLoadDocument(*doc) )
+    {
+        // Still avoid memory leaks.
+        delete doc;
+        return false;
+    }
+
+    // We need to use something instead of the file name, so if we were not
+    // given a name synthesize something ourselves.
+    wxString docname = name;
+    if ( docname.empty() )
+    {
+        static unsigned long s_xrcDocument = 0;
+
+        // Make it look different from any real file name.
+        docname = wxString::Format(wxS("<XML document #%lu>"), ++s_xrcDocument);
+    }
+
+    Data().push_back(new wxXmlResourceDataRecord(docname, doc, XRCWhence::From_Doc));
+
+    return true;
 }
 
 wxXmlNode *wxXmlResource::DoFindResource(wxXmlNode *parent,

tests/xml/xrctest.cpp

@@ -20,13 +20,18 @@
 #if wxUSE_XRC
 
 #include "wx/fs_inet.h"
+#include "wx/imagxpm.h"
 #include "wx/xml/xml.h"
+#include "wx/scopedptr.h"
 #include "wx/sstream.h"
 #include "wx/wfstream.h"
 #include "wx/xrc/xmlres.h"
+#include "wx/xrc/xh_bmp.h"
 
 #include <stdarg.h>
 
+#include "testfile.h"
+
 // ----------------------------------------------------------------------------
 // helpers to create/save some xrc
 // ----------------------------------------------------------------------------
@@ -36,9 +41,19 @@ namespace
 
 static const char *TEST_XRC_FILE = "test.xrc";
 
+void LoadXrcFrom(const wxString& xrcText)
+{
+    wxStringInputStream sis(xrcText);
+    wxScopedPtr<wxXmlDocument> xmlDoc(new wxXmlDocument(sis, "UTF-8"));
+    REQUIRE( xmlDoc->IsOk() );
+
+    // Load the xrc we've just created
+    REQUIRE( wxXmlResource::Get()->LoadDocument(xmlDoc.release(), TEST_XRC_FILE) );
+}
+
 // I'm hard-wiring the xrc into this function for now
 // If different xrcs are wanted for future tests, it'll be easy to refactor
-void CreateXrc()
+void LoadTestXrc()
 {
     const char *xrcText =
     "<?xml version=\"1.0\" ?>"
@@ -116,13 +131,7 @@ void CreateXrc()
     "</resource>"
       ;
 
-    // afaict there's no elegant way to load xrc direct from a string
+    LoadXrcFrom(wxString::FromAscii(xrcText));
-    // So save it as a file, from which it can be loaded
-    wxStringInputStream sis(xrcText);
-    wxFFileOutputStream fos(TEST_XRC_FILE);
-    REQUIRE(fos.IsOk());
-    fos.Write(sis);
-    REQUIRE(fos.Close());
 }
 
 } // anon namespace
@@ -135,8 +144,7 @@ void CreateXrc()
 class XrcTestCase
 {
 public:
-    XrcTestCase() { CreateXrc(); }
+    XrcTestCase() { }
-    ~XrcTestCase() { wxRemoveFile(TEST_XRC_FILE); }
 
 private:
     wxDECLARE_NO_COPY_CLASS(XrcTestCase);
@@ -148,8 +156,7 @@ TEST_CASE_METHOD(XrcTestCase, "XRC::ObjectReferences", "[xrc]")
 
     for ( int n = 0; n < 2; ++n )
     {
-        // Load the xrc file we're just created
+        LoadTestXrc();
-        REQUIRE( wxXmlResource::Get()->Load(TEST_XRC_FILE) );
 
         // In xrc there's now a dialog containing two panels, one an object
         // reference of the other
@@ -171,8 +178,7 @@ TEST_CASE_METHOD(XrcTestCase, "XRC::IDRanges", "[xrc]")
     // Tests ID ranges
     for ( int n = 0; n < 2; ++n )
     {
-        // Load the xrc file we're just created
+        LoadTestXrc();
-        REQUIRE( wxXmlResource::Get()->Load(TEST_XRC_FILE) );
 
         // foo[start] should == foo[0]
         CHECK( XRCID("SecondCol[start]") == XRCID("SecondCol[0]") );
@@ -205,6 +211,52 @@ TEST_CASE_METHOD(XrcTestCase, "XRC::IDRanges", "[xrc]")
     }
 }
 
+TEST_CASE("XRC::PathWithFragment", "[xrc][uri]")
+{
+    wxXmlResource::Get()->AddHandler(new wxBitmapXmlHandler);
+    wxImage::AddHandler(new wxXPMHandler);
+
+    const wxString filename = "image#1.xpm";
+    TempFile xpmFile(filename);
+
+    // Simplest possible XPM, just to have something to create a bitmap from.
+    static const char* xpm =
+        "/* XPM */\n"
+        "static const char *const xpm[] = {\n"
+        "/* columns rows colors chars-per-pixel */\n"
+        "\"1 1 1 1\",\n"
+        "\"  c None\",\n"
+        "/* pixels */\n"
+        "\" \"\n"
+        ;
+
+    wxFFile ff;
+    REQUIRE( ff.Open(filename, "w") );
+    REQUIRE( ff.Write(wxString::FromAscii(xpm)) );
+    REQUIRE( ff.Close() );
+
+    // Opening a percent-encoded URI should work.
+    wxString url = filename;
+    url.Replace("#", "%23");
+
+    LoadXrcFrom
+    (
+        wxString::Format
+        (
+            "<?xml version=\"1.0\" ?>"
+            "<resource>"
+            "  <object class=\"wxBitmap\" name=\"bad\">%s</object>"
+            "  <object class=\"wxBitmap\" name=\"good\">%s</object>"
+            "</resource>",
+            filename,
+            url
+        )
+    );
+
+    CHECK( wxXmlResource::Get()->LoadBitmap("good").IsOk() );
+    CHECK( !wxXmlResource::Get()->LoadBitmap("bad").IsOk() );
+}
+
 // This test is disabled by default as it requires the environment variable
 // below to be defined to point to a HTTP URL with the file to load.
 //