clonemeagain / attachment_preview

osTicket Plugin: Allows inline view of attachments
GNU General Public License v2.0
48 stars 16 forks source link

[compat] register context problem debug_log reference for php olders #26

Closed mckaygerhard closed 6 years ago

mckaygerhard commented 6 years ago

hi @clonemeagain sorry for againt with the backguard compatibility, i'm have a very special case in my job, due we are under a very hard crissis and i have a xtrange idea.. hard to explain but there its the problem:

i using php 5.3 and seems the tests was agains 5.4 and in production we have 5.3 patched with security fixeds.. i have some array short way problems, solved with the attached patch i supply here.. support.php5.3.diff.txt (how stupid, github dont support a diff file!! stupid windosers)

after right array convert to php 5.3 old way, got that problem that seems are a context problem:

Fatal error: Using $this when not in object context 
in /srv/intranet/elticket/include/plugins/attachment_preview/class.AttachmentPreviewPlugin.php 
on line 107 

i dont understang so much about abstraction programing but: the context was changed due the call function here are not from instance in register function?..

mckaygerhard commented 6 years ago

oh! after hard reading, since PHP 5.4 $this can be used in anonymous functions and it refers to the current object, arrggggg damn! so then i must use $selt but i dont know how made it!

mckaygerhard commented 6 years ago

arrggg cannot access self when no class scope is active pufff

clonemeagain commented 6 years ago

Hmm, self won't work because the methods we are calling are not static, you'd need to use $this->{$method}($dom,$link). Or type it all out like:

                    $func = $allowed_extensions[$ext];
                    if (method_exists($this, $func)) {
                        switch ($func) {
                            case 'add_pdf': $this->add_pdf($dom, $link);
                                break;
                            case 'add_text': $this->add_text($dom, $link);
                                break;
                            case 'add_html': $this->add_html($dom, $link);
                                break;
                            case 'add_image': $this->add_image($dom, $link);
                                break;
                            case 'add_audio': $this->add_audio($dom, $link);
                                break;
                            case 'add_video': $this->add_video($dom, $link);
                                break;
                            default: throw new \Exception("Invalid function");
                        }
                    }

Which you can see in the youtube part beneath. I'm in favour of the former. Added patch to new branch so you can test that branch.

mckaygerhard commented 6 years ago

with the last commits are working with php 5.4 and 7.0, but with 5.3 still not, see the comments at the pull #27

clonemeagain commented 6 years ago

Ok, found a few issues..

  1. The DOMDocument::loadHtml function was erroring on invalid parameters, thanks to a 5.4 change
  2. The DOMDocument::findElementsByTagName function doesn't seem to work in 5.3, changed to a DOMXPath::query, and away she goes.
  3. Added more logging to see it working to help locate those issues.

osTicket 1.9 is working on my dev box with PHP5.3!

mckaygerhard commented 6 years ago

hi @clonemeagain great thanks and i have couple of cuestions as i posted in the pull, i also have mi 1.9 working both 5.3 and 5.4, found only one of those problems, gone away with lasted commit c40bc1a8aba7005cb3eedaaa2bba111098dd29de of the pull #27 , now i studing that on each "link search" of the plugin i must detect why my path of popup windows for attachment make this plugin to not work.. i'll explain, i mean i made a if-else that detects that, i'm currently working on that but any other help are welcome, the pacht of popup window are that:

@@ -653,7 +653,7 @@ class ThreadEntry {
            if($attachment['size'])
                $size=sprintf('<em>(%s)</em>', Format::file_size($attachment['size']));

-            $str.=sprintf('<a class="Icon file no-pjax" href="%s" target="%s">%s</a>%s&nbsp;%s',
+            $str.=sprintf('<a class="Icon file no-pjax" href="%s" target="_blank" onClick="window.open(this.href, this.target, \'width=900,height=700\'); return false;">%s%s&nbsp;%s</a>',
                    $attachment['download_url'], $target, Format::htmlchars($attachment['name']), $size, $separator);
        }
mckaygerhard commented 6 years ago

fixed, my pacht was wrong.. i'll explain in the pull.. corrected by reading your excelent comments, a extra point in the a tag atrribute was the problem..:

@@ -653,7 +653,7 @@ class ThreadEntry {
            if($attachment['size'])
                $size=sprintf('<em>(%s)</em>', Format::file_size($attachment['size']));

-            $str.=sprintf('<a class="Icon file no-pjax" href="%s" target="%s">%s</a>%s&nbsp;%s',
+            $str.=sprintf('<a class="Icon file no-pjax" href="%s" target="_blank" onClick="window.open(this.href, this.target, \'width=900,height=700\'); return false;">%s</a>%s&nbsp;%s',
                    $attachment['download_url'], $target, Format::htmlchars($attachment['name']), $size, $separator);
        }
clonemeagain commented 6 years ago

Fixed with last merge, still need to update the readme!

mckaygerhard commented 6 years ago

please let me redact it! (trust me i write with good spell)

mckaygerhard commented 6 years ago

i closed this issue due e5d6baf7b4cf026e8443bfdd8de69563f96de8a5 ... now let me made a pull with some documentation...