Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

attachment_preview breaks jQuery #41

Closed
stevland opened this issue Jun 20, 2019 · 7 comments
Closed

attachment_preview breaks jQuery #41

stevland opened this issue Jun 20, 2019 · 7 comments

Comments

@stevland
Copy link

Hi clonemeagain,

I have a couple osTicket Awesome users that are also using your plugin. When your plugin is enabled some jQuery becomes borked in an odd way.

Any change you might know what is going on, off the top of your head?

I'd prefer to find a fix rather than asking my users to disable your plugin. Cheers!

@clonemeagain
Copy link
Owner

Haven't tried it, can you capture the console log?

Off the top of my head? I'd think it might be changing the files the plugin assumes are there in ways it doesn't anticipate.. might have to get a copy and have a look

@stevland
Copy link
Author

stevland commented Jun 20, 2019

That's weird, I wasn't seeing anything significant in the console earlier, but now I am:

Uncaught SyntaxError: Invalid or unexpected token
tickets.php?queue=1:1059 AP: Plugin running, initial fetch limit configured to ALL.
tickets.php?queue=1:1233 AP: Plugin loaded

I set up a demo installation with the plugin activated if you would like to have a look.

https://osticketawesome.com/a/1.12-4-attach-preview/scp/

user: demo
pass: demodemo

The malformed jQuery doesn't show up on every ticket page, but it does on the Answered page, for example.

@stevland
Copy link
Author

It is rather fascinating. When viewing an affected page in mobile layout, some of the jQuery is even rendering within the mangled code.

@clonemeagain
Copy link
Owner

I think it must be the XML processing.. the plugin converts the page output into a DOMDocument, then spits it back out as HTML. Normally OST is a well-formed document, and the xml stuff is configured to ignore trivial errors, so this works. I guess they are doing something "technically possible", but not within spec, so it breaks when converted like that. I can see jQuery and HTML merged together, likely because the jQuery is supposed to be writing HTML, however after conversion is quoted wrong, so get's all munged in the browser.

If you want, you could try tweaking these settings: https://github.com/clonemeagain/attachment_preview/blob/master/class.AttachmentPreviewPlugin.php#L655 and see if that helps. Without the theme source I couldn't say for sure, but I would like to pressure them to rewrite so many inlined scripts as a single file which would bypass any of those problems.

I'm utterly swamped at the moment, but when I get a chance, I'll test the theory more and see what I can do on the plugins side, perhaps it needs a rewrite to bypass the DOMDocument entirely.

@clonemeagain
Copy link
Owner

Alright, just playing with the options, if you leave the plugin enabled, but disable all the "allow embed PDF as inline" and such, turning them all off, it renders the other script quotes correctly. Enable even one of them (even if not used), and it changes the quotes from single to double in the theme scripts. That is weird.

Doesn't affect all the scripts either, just this one:

<script>
--
  $( "table.list.queue.tickets a.preview" ).wrap( "<div class='ticket-num'></script></div>" );
   $( "th.osta_ticket a" ).text( "#" );
   $( "div[style='font-weight:bold']" ).closest( "td" ).addClass( "new-reply-waiting" );
   $( "td.osta_ticket" ).prepend( "<div id="new-reply-icon" data-placement="top" data-toggle="tooltip" data-original-title="New Reply"><span class="dot"></span></div>" );
    
   function myFunction(x) {
   if (x.matches) { // If media query matches
   $( "tbody tr" ).wrapInner( "<label></label>" );
   $( "td.osta_priority.osta_priority_low" ).closest( "tr" ).addClass( "priority-low" );
   $( "td.osta_priority.osta_priority_normal" ).closest( "tr" ).addClass( "priority-normal" );
   $( "td.osta_priority.osta_priority_high" ).closest( "tr" ).addClass( "priority-high" );
   $( "td.osta_priority.osta_priority_emergency" ).closest( "tr" ).addClass( "priority-emerency" );
    
   $( ".overdueTicket" ).closest( "tr" ).addClass( "overdue" );
   $( ".overdue td.osta_ticket" ).append( "<div class="overdue-ticket" data-placement="top" data-toggle="tooltip" data-original-title="Ticket is Overdue!"> </div>" );
    
   $( ".paperclip" ).closest( "tr" ).addClass( "paperclip-icon" );
   $( ".paperclip-icon td.osta_ticket" ).append( "<div class="ticket-has-attachement" data-placement="top" data-toggle="tooltip" data-original-title="Ticket Has Attachment"> </div>" );
   $( "tr" ).each(function() {
   var $this = $(this),
   $href = $this.find( ".pull-right" ),
   $target = $this.find( "td.osta_ticket" );
   $href.appendTo($target);
   });
   } else {
   ;
   }
   }
   var x = window.matchMedia( "(max-width: 760px)" )
   myFunction(x) // Call listener function at run time
   x.addListener(myFunction) // Attach listener function on state changes
   jQuery( ".truncate" ).each(function(i, value) {
   var $link = jQuery(value);
   var text = $link.text();
   if(text.length &gt; 55) {
   $link.text(text.substring(0, 55) + "...");
   }
   });

It's right before /css/filedrop.css above the footer, also, uses ES6 style line termination in the matchMedia bits.. weird.

@mckaygerhard
Copy link
Contributor

hi.. osticket currently closed lot of issues.. but a special one was taked in consideration: "Validations on forms not permit "0" as value" the number 1996 i not linked here due has lot of activity .. but i posted a note about the running behaviour of "updates and updates of changes" ...

@stevland
Copy link
Author

Hi clonemeagain, it appears that this incompatibility no longer exists in the latest version of attachment_preview (v1.11). If you tweaked something, I appreciate it. Either way, amazing plugin, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants