From c16082edfee008f9a9d85dd0b71aa02934740fcd Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 14 Apr 2021 23:59:39 +0100 Subject: [PATCH 1/5] Add wxXmlResource::LoadDocument() This allows loading XRC from anywhere, not just files and URLs. --- include/wx/xrc/xmlres.h | 10 +++++ interface/wx/xrc/xmlres.h | 48 +++++++++++++++++++++++- src/xrc/xmlres.cpp | 78 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 129 insertions(+), 7 deletions(-) diff --git a/include/wx/xrc/xmlres.h b/include/wx/xrc/xmlres.h index 6507afe6d9..603b35eff4 100644 --- a/include/wx/xrc/xmlres.h +++ b/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(); diff --git a/interface/wx/xrc/xmlres.h b/interface/wx/xrc/xmlres.h index 9feca5980f..9a87d7db3c 100644 --- a/interface/wx/xrc/xmlres.h +++ b/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 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); diff --git a/src/xrc/xmlres.cpp b/src/xrc/xmlres.cpp index e6248d4716..65918e929f 100644 --- a/src/xrc/xmlres.cpp +++ b/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 " ); - 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(""), ++s_xrcDocument); + } + + Data().push_back(new wxXmlResourceDataRecord(docname, doc, XRCWhence::From_Doc)); + + return true; } wxXmlNode *wxXmlResource::DoFindResource(wxXmlNode *parent, From ebedd023f5f3d7e420e3723e2a5789defad8d347 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 15 Apr 2021 00:01:44 +0100 Subject: [PATCH 2/5] Use wxXmlResource::LoadDocument() in XRC unit tests This is simpler and better than creating a temporary file unnecessarily and also serves as a test of the just added function. --- tests/xml/xrctest.cpp | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/tests/xml/xrctest.cpp b/tests/xml/xrctest.cpp index 04e1a435ff..196bd1637a 100644 --- a/tests/xml/xrctest.cpp +++ b/tests/xml/xrctest.cpp @@ -21,6 +21,7 @@ #include "wx/fs_inet.h" #include "wx/xml/xml.h" +#include "wx/scopedptr.h" #include "wx/sstream.h" #include "wx/wfstream.h" #include "wx/xrc/xmlres.h" @@ -36,9 +37,19 @@ namespace static const char *TEST_XRC_FILE = "test.xrc"; +void LoadXrcFrom(const wxString& xrcText) +{ + wxStringInputStream sis(xrcText); + wxScopedPtr 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 = "" @@ -116,13 +127,7 @@ void CreateXrc() "" ; - // afaict there's no elegant way to load xrc direct from a string - // 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()); + LoadXrcFrom(wxString::FromAscii(xrcText)); } } // anon namespace @@ -135,8 +140,7 @@ void CreateXrc() class XrcTestCase { public: - XrcTestCase() { CreateXrc(); } - ~XrcTestCase() { wxRemoveFile(TEST_XRC_FILE); } + XrcTestCase() { } private: wxDECLARE_NO_COPY_CLASS(XrcTestCase); @@ -148,8 +152,7 @@ TEST_CASE_METHOD(XrcTestCase, "XRC::ObjectReferences", "[xrc]") for ( int n = 0; n < 2; ++n ) { - // Load the xrc file we're just created - REQUIRE( wxXmlResource::Get()->Load(TEST_XRC_FILE) ); + LoadTestXrc(); // In xrc there's now a dialog containing two panels, one an object // reference of the other @@ -171,8 +174,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 - REQUIRE( wxXmlResource::Get()->Load(TEST_XRC_FILE) ); + LoadTestXrc(); // foo[start] should == foo[0] CHECK( XRCID("SecondCol[start]") == XRCID("SecondCol[0]") ); @@ -205,6 +207,19 @@ TEST_CASE_METHOD(XrcTestCase, "XRC::IDRanges", "[xrc]") } } +TEST_CASE("XRC::PathWithFragment", "[xrc][uri]") +{ + LoadXrcFrom + ( + "" + "" + " bitmap#1.png" + "" + ); + + CHECK( wxXmlResource::Get()->LoadBitmap("bitmap").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. // From 97750eb282d3aa6197e9f2ecc9eb6bcd224c0863 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 15 Apr 2021 18:20:32 +0100 Subject: [PATCH 3/5] Don't try relative paths in wxFileSystem if current path is empty This seems redundant, as the loop is the same as the one just below it which is executed in any case if this one doesn't find anything. --- src/common/filesys.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/filesys.cpp b/src/common/filesys.cpp index 2e2337a198..a81be7ba77 100644 --- a/src/common/filesys.cpp +++ b/src/common/filesys.cpp @@ -478,7 +478,7 @@ wxFSFile* wxFileSystem::OpenFile(const wxString& location, int flags) m_LastName.clear(); // try relative paths first : - if (meta != wxT(':')) + if (meta != wxT(':') && !m_Path.empty()) { node = m_Handlers.GetFirst(); while (node) From b9bd9320772ce8af01adfd284be62f30290e831d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 15 Apr 2021 18:22:16 +0100 Subject: [PATCH 4/5] Micro optimize string concatenation in wxFileSystem::OpenFile() Concatenate the string only once instead of doing it several times. Compiler might be optimizing this anyhow in release builds, but it definitely helps in at least the debug ones and doesn't cost anything. --- src/common/filesys.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/common/filesys.cpp b/src/common/filesys.cpp index a81be7ba77..42b2a66717 100644 --- a/src/common/filesys.cpp +++ b/src/common/filesys.cpp @@ -480,14 +480,15 @@ wxFSFile* wxFileSystem::OpenFile(const wxString& location, int flags) // try relative paths first : 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); - if (s) { m_LastName = m_Path + loc; break; } + s = MakeLocal(h)->OpenFile(*this, fullloc); + if (s) { m_LastName = fullloc; break; } } node = node->GetNext(); } From 2babb9e06cbaa28bdbd7cf290ebf7f2ba5c0db4e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 15 Apr 2021 18:49:16 +0100 Subject: [PATCH 5/5] Document and test that bitmap paths in XRC are percent-encoded These paths are actually URLs and so the special URL characters must be percent-encoded in them. Document this and add a test checking that this is how it works. Closes #19142. --- docs/doxygen/overviews/xrc_format.h | 14 +++++++-- tests/xml/xrctest.cpp | 47 ++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/docs/doxygen/overviews/xrc_format.h b/docs/doxygen/overviews/xrc_format.h index a23fba23d1..448c32c304 100644 --- a/docs/doxygen/overviews/xrc_format.h +++ b/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 -wxFileSystem URL) of the bitmap to use. For example: +basic form, their text value is simply a relative URL of the bitmap to use. +For example: @code New @@ -350,7 +350,15 @@ wxFileSystem URL) of the bitmap to use. For example: @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 + + images/%231/tool.png + +@endcode Bitmap file paths can include environment variables that are expanded if wxXRC_USE_ENVVARS was passed to the wxXmlResource constructor. diff --git a/tests/xml/xrctest.cpp b/tests/xml/xrctest.cpp index 196bd1637a..d5a0ee0514 100644 --- a/tests/xml/xrctest.cpp +++ b/tests/xml/xrctest.cpp @@ -20,14 +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 +#include "testfile.h" + // ---------------------------------------------------------------------------- // helpers to create/save some xrc // ---------------------------------------------------------------------------- @@ -209,15 +213,48 @@ 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 ( - "" - "" - " bitmap#1.png" - "" + wxString::Format + ( + "" + "" + " %s" + " %s" + "", + filename, + url + ) ); - CHECK( wxXmlResource::Get()->LoadBitmap("bitmap").IsOk() ); + CHECK( wxXmlResource::Get()->LoadBitmap("good").IsOk() ); + CHECK( !wxXmlResource::Get()->LoadBitmap("bad").IsOk() ); } // This test is disabled by default as it requires the environment variable