Please review the discussion in #488300: Pathologic screws up paths given using tokens.

When you have text that uses tokens and the text also needs to be run through an input filter, the tokens should be processed first so that the input filter can operate on the results.

In the bug report referenced, the pathologic filter changes href="[order-url]" into href="https://somesite.com/[order-url]" which then becomes "https://somesite.com/https://somesite.com/...". Other filters may also fail to work properly.

CommentFileSizeAuthor
#7 uc_cart.module.patch2.08 KBanrikun
#4 uc_cart.module.patch2.13 KBanrikun

Comments

donquixote’s picture

donquixote commented

subscribe

tr’s picture

tr commented
Version: 6.x-2.0-rc3 » 6.x-2.x-dev

OK, I read the issue you referred to, and I see what you mean. Can you point out (preferably with a patch) exactly what changes you propose to make in Ubercart to fix the problem?

anrikun’s picture

anrikun commented

+1 Subscribing
I'm having the same issue (without using Pathologic).

Also, arbitrarily wrapping <p> tags around filtered texts without knowing their contents is not a good idea as it might result in invalid XHTML.
(Seen in uc_cart.module uc_cart_complete_sale() function)

anrikun’s picture

anrikun commented
Assigned: Unassigned » anrikun
Status: Active » Needs review
StatusFileSize
new uc_cart.module.patch2.13 KB

Here's a patch that fixes this issue.

For compatibility, I have not changed the <p> tags.

This patch is for 6.x-2.2.

tr’s picture

tr commented

Is that the only place that this has to be fixed, or does this problem also occur elsewhere in Ubercart?

anrikun’s picture

anrikun commented

Yes it is the only place.

anrikun’s picture

anrikun commented
StatusFileSize
new uc_cart.module.patch2.08 KB

Finally, I have removed the <p> tags as they should definitely not be added.
Here is the updated patch.

This patch is for 6.x-2.2.

opteronmx’s picture

opteronmx commented

#7 Confirmed, Applied and Tested, appears to work well.

Thanks anrikun!

anrikun’s picture

anrikun commented
Status: Needs review » Reviewed & tested by the community

Then I guess we can update this issue's status.
Thanks for your review!

Island Usurper’s picture

Island Usurper commented
Status: Reviewed & tested by the community » Fixed

I was a little concerned that without the <p> tags, all of the messages would run together. But it looks like the filters will be taking care of that automatically.

So, thanks for the patch! Committed to DRUPAL-6--2.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.