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

[2.5] Change the cloak container from div to span #3955

Merged
merged 1 commit into from Jul 24, 2014

Conversation

Projects
None yet
6 participants
@mbabker
Copy link
Member

mbabker commented Jul 24, 2014

Ref #3953

@dbhurley

This comment has been minimized.

Copy link
Contributor

dbhurley commented Jul 24, 2014

+1 works here. One more test?

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Jul 24, 2014

OK, for me
Making PR for loss of class

dbhurley pushed a commit that referenced this pull request Jul 24, 2014

David Hurley
Merge pull request #3955 from joomla/2.5-cloakDiv
[2.5] Change the cloak container from div to span

@dbhurley dbhurley merged commit f280f8b into 2.5.x Jul 24, 2014

@mbabker mbabker deleted the 2.5-cloakDiv branch Jul 24, 2014

@RedEvo

This comment has been minimized.

Copy link

RedEvo commented Jul 30, 2014

A post 2.5.24 upgrade on our site gave this:
https://www.dropbox.com/s/tj715hf9seyfa3q/Screenshot%202014-07-30%2006.29.02.png
Unpublishing the cloaking plugin fixed it. I'm assuming others will have this issue.

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Jul 30, 2014

@RedEvo
What was exactly the code used?

Please test #3967

@MagnumGR

This comment has been minimized.

Copy link

MagnumGR commented Jul 30, 2014

I have the same problem with RedEvo with one of my sites.
The only solution is unpublishing the cloacking plugin.

@RedEvo

This comment has been minimized.

Copy link

RedEvo commented Jul 30, 2014

Code Generated with cloaking plugin unpublished

<div class="contact email"><a href="mailto:info@redevolution.com"><img style="margin: 0px 3px 0px 0px; float:left; width:21px; height:21px;" title="Email Red Evolution" src="/images/stories/icons/social_icons/icon_email.png" alt="Email Red Evolution" width="21" height="21" /> Email Us</a></div> 

Code Generated with cloaking plugin published (rogue span tag?)

<span class="contact email"><a href="mailto:<span id="cloak47030">This email address is being protected from spambots. You need JavaScript enabled to view it.</span><script type='text/javascript'>
 <!--
 document.getElementById('cloak47030').innerHTML = '';
 var prefix = '&#109;a' + 'i&#108;' + '&#116;o';
 var path = 'hr' + 'ef' + '=';
 var addy47030 = '&#105;nf&#111;' + '&#64;';
 addy47030 = addy47030 + 'r&#101;d&#101;v&#111;l&#117;t&#105;&#111;n' + '&#46;' + 'c&#111;m';
 document.getElementById('cloak47030').innerHTML += '<a ' + path + '\'' + prefix + ':' + addy47030 + '\'>' + addy47030+'<\/a>';
 //-->\n </script>"><img style="margin: 0px 5px 0px 0px; float: left; width: 27px; height: 27px;" title="Email Red Evolution" src="/images/stories/icons/social_icons/icon_email.png" alt="Email Red Evolution" width="27" height="27" /> <span id="cloak9209">This email address is being protected from spambots. You need JavaScript enabled to view it.</span><script type='text/javascript'>
 <!--
 document.getElementById('cloak9209').innerHTML = '';
 var prefix = '&#109;a' + 'i&#108;' + '&#116;o';
 var path = 'hr' + 'ef' + '=';
 var addy9209 = '&#105;nf&#111;' + '&#64;';
 addy9209 = addy9209 + 'r&#101;d&#101;v&#111;l&#117;t&#105;&#111;n' + '&#46;' + 'c&#111;m';
 document.getElementById('cloak9209').innerHTML += '<a ' + path + '\'' + prefix + ':' + addy9209 + '\'>' + addy9209+'<\/a>';
 //-->\n </script></a></span>
@Bakual

This comment has been minimized.

Copy link
Contributor

Bakual commented Jul 30, 2014

Did you try if the PR #3967 fixes your issue?

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Jul 31, 2014

This code will never work and did NOT work on a 2.5.19 when cloaked (i.e. before the patch), even if patching #3967 OR in 3.3.3.
It contains at the same time an image and a text. If you change it to (i.e. taking off the /> Email Us )

<div class="contact email"><a href="mailto:info@redevolution.com"><img style="margin: 0px 3px 0px 0px; float:left; width:21px; height:21px;" title="Email Red Evolution" src="/images/stories/icons/social_icons/icon_email.png" alt="Email Red Evolution" width="21" height="21"</a></div> 

It will work OK after patch on 2.5.x or in 3.3.3

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Jul 31, 2014

Preparing patch to allow:

/*
         * Search for derivatives of link code <a href="mailto:email@example.org">
         * <img anything>any text</a>
         */

and

/*
         * Search for derivatives of link code
         * <a href="mailto:email@amail.com?subject=Text"><img anything>any text</a>
         */

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Jul 31, 2014

Please test (on a 3.3.3)
#4041

@RedEvo

This comment has been minimized.

Copy link

RedEvo commented Jul 31, 2014

Sorry, I'm a little confused now. The cloaking worked fine with version 2.5.23 and failed when upgrading to 2.5.24. If you are suggesting the code never worked then I'm confused as the error appeared post 2.5.23 to 2.5.24 upgrade.
The HTML that's displayed with cloaking turned off is:
https://www.dropbox.com/s/3h9pyu4p4xzxodf/Screenshot%202014-07-31%2009.18.24.png

<span class="contact email"><a href="mailto:info@redevolution.com"><img style="margin: 0px 5px 0px 0px; float: left; width: 27px; height: 27px;" title="Email Red Evolution" src="/images/stories/icons/social_icons/icon_email.png" alt="Email Red Evolution" width="27" height="27" /> info@redevolution.com</a></span>

With cloaking turned on the HTML becomes invalid.
https://www.dropbox.com/s/c3o3plpmm4lc3fp/Screenshot%202014-07-31%2009.20.30.png

<span class="contact email"><a href="mailto:<span id="cloak44622">This email address is being protected from spambots. You need JavaScript enabled to view it.</span><script type='text/javascript'>
 <!--
 document.getElementById('cloak44622').innerHTML = '';
 var prefix = '&#109;a' + 'i&#108;' + '&#116;o';
 var path = 'hr' + 'ef' + '=';
 var addy44622 = '&#105;nf&#111;' + '&#64;';
 addy44622 = addy44622 + 'r&#101;d&#101;v&#111;l&#117;t&#105;&#111;n' + '&#46;' + 'c&#111;m';
 document.getElementById('cloak44622').innerHTML += '<a ' + path + '\'' + prefix + ':' + addy44622 + '\'>' + addy44622+'<\/a>';
 //-->\n </script>"><img style="margin: 0px 5px 0px 0px; float: left; width: 27px; height: 27px;" title="Email Red Evolution" src="/images/stories/icons/social_icons/icon_email.png" alt="Email Red Evolution" width="27" height="27" /> <span id="cloak14236">This email address is being protected from spambots. You need JavaScript enabled to view it.</span><script type='text/javascript'>
 <!--
 document.getElementById('cloak14236').innerHTML = '';
 var prefix = '&#109;a' + 'i&#108;' + '&#116;o';
 var path = 'hr' + 'ef' + '=';
 var addy14236 = '&#105;nf&#111;' + '&#64;';
 addy14236 = addy14236 + 'r&#101;d&#101;v&#111;l&#117;t&#105;&#111;n' + '&#46;' + 'c&#111;m';
 document.getElementById('cloak14236').innerHTML += '<a ' + path + '\'' + prefix + ':' + addy14236 + '\'>' + addy14236+'<\/a>';
 //-->\n </script></a></span>

Which patch would you like us to try on 2.5.24?
many thanks
d

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Jul 31, 2014

I am saying that the code you used could not be cloaked correctly pre 2.5.23 ( I tested), because the case was not taken care of in the plugin ( #3955 (comment) )

What I suggest, until a full 2.5 patch is made ( #3967 does NOT include yet the new cases) is to install a 3.3.3 and test, then patch with #4041 and retest.

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Jul 31, 2014

Btw, in your original post you had

[...]
height="21" /> Email Us</a></div> 

@RedEvo

This comment has been minimized.

Copy link

RedEvo commented Jul 31, 2014

Yes I know. We have different versions for different view ports :) Last time a pasted the wrong one - but the same problem. We're rebuilding using 3.3.3 just now so we'll live with the issue however we have many sites to upgrade to 2.5.24 which we'll leave just now, perhaps 2.5.25 is on its way :)

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Jul 31, 2014

Well it is not the same problem. That case ( height="27" /> info@redevolution.com )would need other cases in #4041

Will look into it.

@infograf768

This comment has been minimized.

Copy link
Member

infograf768 commented Aug 1, 2014

Added the 2 cases in #4041
Need testers...

@Bakual

This comment has been minimized.

Copy link
Contributor

Bakual commented Aug 1, 2014

Please stop commenting on an already closed issue.
Please test the referenced PR and comment there if there is still an issue after applying it.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.