Malformed HTML in long history

1

We are using v5.0.3 and seeing some oddness in ticket history display. Wondering if anyone else has seen similar behavior and/or has suggestions for remedy.

Short version…
Tickets with long history, at a certain point (I haven’t gone to the trouble of figuring out if the break is consistent across tickets) history transactions fall outside of the collapsable div element. Then, if the history continues long enough after that, transactions start to lose formatting. This seems to be because they are another level up in the HTML, and so lose a css class attribute defined in a div.

In case my description is opaqaue:

<div> <!--Element has css attribute-->
	<div> <!--collapse target-->
	   Transactions here operate as expected.
	</div>
	Transactions here are correctly formatted but don't collapse.
</div>
Transaction here don't collapse, and are oddly formatted.

The root cause seems to be malformed AJAX coming from the server which is then inserted into the history target.

Another interesting twist. In mid-January we upgraded from v3.6.11. To date I have not found any tickets created prior to the updgrade that show this behavior.

Thoughts?

2

I reported this bug to rt-bugs (at) bestpractical (dot) com on 20 Nov 2018 and did not receive an acknowledgement of any kind. I would love to know what the correct way to report bugs in RT is, because the published method (their RT instance) seems to be full of spam and not recently touched by developers.

So, this bug has existed for a long time. We had RT 4.4.1 at that time, later upgraded to 4.4.3. We have just (belatedly) installed 5.0.3 and it does suffer from the same thing, although the appearance is slightly different because of the differing themes.

It appears to be down to RT’s processing of transactions that come in via HTML email and have a lot of nested quoted material in them, and I have an HTML example that can reproduce the bug. For each one of these transactions, RT emits an extra “</div>” and this, as you say, makes the formatting come out wrong. In the default RT theme an immediate consequence of this, if the preference “show history immediately” is set, is that the grey RT footer, that should come at the end of the document, instead comes at the bottom of the first screen and covers up some of the UI elements.

I will gladly send the reproducer to someone at RT if they can explain how best to make contact.

3

We have a branch that might fix this issue. You could patch your system with this update and let us know if it solves the issue:

4

Thanks. I’ll take a look.

5

Thanks Jim_Brandt. I have not applied that, but I do believe it would fix the exact issue we are seeing although it looks kinda hacky (it depends on the exact structure emitted by FCK). My alternative fix would be to simply not do Outlook processing unless either Depth==0 or ContentType==text/plain.

This arises when an HTML user replies to a plain-text Outlook user who replied to an earlier email, both replies having been top-posted. When Outlook is set to plain text, a reply from that person will be of Content-Type text/plain and consist of (a) their text; (b) the separator (“-----Original Message-----”), and (c) the entire content of the email they replied to. When an HTML user replies to that, what’s appended to their reply looks like <blockquote><pre> (text of the previous message) </pre><blockquote>.

RT uses HTML::Quoted to split each transaction into stanzas, but that module doesn’t handle the Outlook separator so as a hack some code has been added to ShowMessageStanza to split the message up at the separator. The problem is that this splits both the <blockquote> and <pre> blocks and the result is an invalidly nested <div>. The browser inserts an extra </div> to close the invalidly nested element before the </pre> close tag; this means there is now an extra </div> at the end of the transaction.

The strictly correct way to fix this would be to submit an enhancement to HTML::Quoted that handle the Outlook separator, instead of trying to fix this on the fly in ShowMessageStanza.

6

Thanks for the feedback on the patch. Your suggestion to potentially move a fix to HTML::Quoted is also reasonable. We’ll take a look.

7

I’ve applied the github patch referenced above to our production RT and it’s working much better with regard to long quoted threads that were being messed up before.

There’s still a case that it doesn’t fix, although this is rarer and it’s a coincidence that it happened to occur now.

If person A forwards a non-RT email to person B, and then person B replies and sends this reply to RT, the structure of the incoming email is now something like:

This is what they typed
<blockquote>
This is what they replied to
<div><br>
-------- Forwarded Message --------
This is the original forwarded email
</div>
</blockquote>

This causes a message-stanza to be inserted at the “Forwarded Message” divider. Without the patch, it copies the </div></blockquote> before closing the inserted message-stanza (so the </div> actually closes the stanza and the </blockquote> and the inserted end-of-stanza are now the wrong way round). With the patch it inserts an extra <blockquote> just inside the stanza but this is still mismatched by the </div> coming from the message.

8

The branch has been updated to handle your example with an unclosed div:

We also added your example as a test case.

I looked at HTML::Quoted and it does seem the special case Outlook code could live there. But looking at the RT side, the current parsing has been in place for a long time (since 2011). For now, I’m inclined to fix this formatting issue at the location of the current parsing code. We could look at refactoring that section into HTML::Quoted as separate future change.