[#5119] - Allow "external" JDocument classes
- Closed
- 20 Nov 2014
- Medium
- Build: staging
- # 5119
- Diff
- bembelimen:patch-23
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
At the moment all JDocument render classes have to be placed into "libraries/joomla/document". You cannot load your own JDocument child class with e.g. a plugin, because if getInstance does not find the class file in the mentioned path, it fallbacks to JDocumentRaw.
With this patch the getInstance method checks first if the class exists (= e.g. loaded by plugin) and uses the JDocumentRaw class as fallback after the requested class does not exist and could not be loaded.
To test: create an own JDocument child class (like this PDF class: http://docs.joomla.org/J2.5:Creating_PDF_views), load it over a plugin and try to call it with "format=pdf" over the url.
Before the patch you should get the raw output, after you should get the PDF.
| Labels |
Added:
?
|
||
Hi, could you provide me a link from the patch you mean? Thanks
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5119.
@bembelimen thats the right one: #4074
I think, you address a different target group than me, but we have the same problem :)
I want to create several components with a PDF export. So I just want to load my JDocumentPdf class without overriding anything. So my patch says: if the class already exists, just take it and don't look if the file is on the correct place.
You on the other hand support the "whole homepage creation devs" who create the template and everything.
So as conclusion I think, the best would be, if we could find a way to merge both ideas:
- if the class exists, just take it
- elseif an override exists take that
- elseif the class file is found in libraries/joomla/document take that
- fallback to raw
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5119.
@bembelimen If we can combine them sounds like the perfect solution!
@dgt41 I created a pull request for your pull request (or something like that...): dgt41#8
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5119.
@bembelimen Thanks! Now lets get some testers
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-11-16 02:47:03 |
| Status | Closed | ⇒ | Pending |
done @dgt41
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5119.
Set to "open" on behalf of @zero-24 by The JTracker Application at issues.joomla.org/joomla-cms/5119
So let's find some tester for this patch ;)
I agree with this PR. Code looks fine.
The current code doesn't make much sense (why load fallback if class exists already?).
To get tests, can you maybe provide a test plugin? That would surely make it easier for non-coders to test this.
| Category | ⇒ | Libraries |
How can I append ZIP-Files to the PR?
GitHub doesn't allow ZIP uploads, you'll need to upload it somewhere else
and share the link here.
On Wednesday, November 19, 2014, bembelimen notifications@github.com
wrote:
How can I append ZIP-Files to the PR?
—
Reply to this email directly or view it on GitHub
#5119 (comment).
Here we go:
http://stuff.benjamintrenkle.de/plg_system_newdocument.zip
- Install
- Activate
- Add "&format=dummy" to your URL
- Before patch: "View not found" error
- After patch: JDocumentDummy loaded
infograf768
- | Milestone |
Added: |
||
| Status | Pending | ⇒ | Closed |
| Closed_Date | 2014-11-16 02:47:03 | ⇒ | 2014-11-20 15:02:09 |
@bembelimen would you like to test if this one solves your problems? Needs one more test...