WordPress.org

1. spherical
   Member
   Posted 2 years ago #

   Okay, I saw this yesterday after proofreading a number of previous posts and it caused me a whole day of grief. Something is going on with AtD where it is doing needless code reformatting, which ends up eventually stripping image tags completely.

   Save a draft with properly formatted tags and it all stays the same when the screen comes back,

   Proofread a post (even without changing any found spelling errors) and the first thing that happens is the img tag gets totally re-written. The closing " /" is stripped out before the > and everything is re-ordered, colors in a style specification are, IMO, needlessly being converted to rgb from hex; adding in a whole bunch of extra characters to specify the same color. #eee is rgb(238, 238, 238) but why change it?

   Additionally,
   margin: 8px 1px 1px 1px;
   gets changed to
   margin: 8px 1px 1px;
   Where's the fourth value?

   margin: 8px 0 0 0;
   becomes
   margin: 8px 0pt 0pt;
   Fourth value is still missing and I suppose that 0px == 0pt but why change that if that is not the way the user prefers to work?

   Saving the draft from there strips out the whole inline style specification! Gone. My guess is that WP doesn't like the rgb and/or the non-standard margin and calls the whole string invalid. This happens with either of these being modified, doesn't have to be both.

   But it gets worse. Doing a lot of research, I found references to kses.php and allowed tags. Checked the new one in 2.8.6 and 'title' in an img tag wasn't there. I added it to the list because I believe that this omission was the further cause of the entire img tag, with fully specified dimensions, border, margins, floats and file location to be reduced to <img /> in every image in the posts that I proofread that morning. Totaled 22 images in 8 posts that I had to find their locations and figure out the code to make the page back into what I had previously crafted.

   Haven't been able to reproduce the entire stripping of the img tag code down to nothing... yet. Trying to reproduce it by adding in a fictitious style spec to take the place of the once-missing 'title' allow, that would cause either AtD or WP to strip it, hasn't done anything unexpected. WP strips it out but the rest is left alone. AtD strips it out but Borks my code in the process and that apparently causes WP to strip the code further when a Save is triggered.

   The above scares the heck out of me enough to disable the plugin until something changes. That was a lot of work that I hadn't planned to do yesterday.

   http://wordpress.org/extend/plugins/after-the-deadline/

2. rsmudge
   Member
   Posted 2 years ago #

   Are you using Internet Explorer? AtD dumps your textarea contents into a DIV, lets you interact with it, and then takes the HTML "source" of the DIV and dumps it back into the textarea when you're done proofreading.

   I know Internet Explorer likes to uppercase tags. I didn't know other transformations were taking place.

3. spherical
   Member
   Posted 2 years ago #

   Nope. FireFox 3.5.6. Only open stuff in IE6, 7 & 8 to debug code.

4. rsmudge
   Member
   Posted 2 years ago #

   I've created a ticket for this issue.
   http://openatd.trac.wordpress.org/ticket/16

   The challenge is I rely on the browser to give me a DOM representation of your HTML and the browser is changing your HTML code to its preferred form. It won't be an overnight fix.

   If you do narrow down code that causes the problem, let me know so I can update the ticket. When I devise a solution I'll test it against what you give me.

5. spherical
   Member
   Posted 2 years ago #

   Thanks! Some additional info: when the first instances occurred, the Visual editor was available. Remembering some posts that said going back and forth between the HTML and Visual editors can mess up your code (why?), I work only in the HTML editor but Visual was available. When all of the stripping was happening, I disabled the Visual Editor to see if that made any difference and left it disabled. For the AtD re-install to get some data for you to work with, the Visual Editor remained disabled. Perhaps this may be a contributing factor to my being unable to reproduce the complete stripping of all specifications within the IMG tags (not just the changing of hex to RGB and subsequent stripping of only the inline style) that I had seen happen previously. I'll make some tests going back and forth between the two editors; leaving AtD disabled, to see what happens.

6. spherical
   Member
   Posted 2 years ago #

   Okay. Going back in and re-enabling the Visual Editor yielded these results:

   Switching back and forth between the two still changes some code.

   hex color shortcut #eee gets lengthened to #eeeeee
   margin:1px 1px 1px 1px; gets shortened to margin: 1px;
   (I get that this is all that's needed in real code--this is a test)
   [caption]...[/caption] gets totally nuked
   some <p>...</p> tags get stripped -- not all -- no idea why

   I, like others who are comfortable with code, am reverting to Visual Editor disabled. It's still broken after all these years. :/

   BUT, the Visual Editor did not alter the hex #eee to rgb(238, 238, 238) which is what seems to trigger the stripping of the entire inline style specifications upon saving.

7. rsmudge
   Member
   Posted 2 years ago #

   Ok, so if I understand correctly, the main difference between what AtD is doing to nuke your code and what the visual editor is doing is AtD is changing hex color values to rgb() values which triggers extra stripping scrutiny from WP?

8. spherical
   Member
   Posted 2 years ago #

   That seems to be it, yes, although I'm not exactly certain what's going on. The stripping happens whether the Visual Editor is enabled/used or not.

9. rsmudge
   Member
   Posted 2 years ago #

   Yesterday I released an updated AtD. One of the fixes in there solves an issue where going from proofreading to editing changes < -> < and > -> > (outside of tags). This may help you.

10. spherical
    Member
    Posted 2 years ago #

    Still doing the same as it was. Just updated to 49006 hoping that things had been addressed.

    Proofread a post, go back to edit, "border:3px double #999;" is changed to: "border:3px double rgb(153, 153, 153);". Update the post, border directive is tripped out entirely. WHY it feels the need to mess with my colors, I have no idea. What's the point of adding characters to a spec when #nnn does the job?

    Deactivated again.

11. spherical
    Member
    Posted 2 years ago #

    Actually, I've tested this more and the whole image style is stripped out but I've learned that the rgb() is what is doing it. I left AtD deactivated and manually went in and edited the original #999 to rgb(153, 153, 153) which AtD insists on putting in there and the whole CSS style spec is gone when the page refreshes. Looked at the kses.php code to see what is going on there, as regards allowed HTML tags and find that each tag is in the form:

    'tag' => array (),

    Hmmm. I'll bet that the () that AtD decides needs to be in there to hold the, IMO, unnecessary rgb values is killing the php eval because you end up with parens inside the php code parens and it barfs. When that happens, WP strips the unrecognized code out!

    Sure enough. Replaced them with [] and the CSS code doesn't get stripped any longer. The image displays as if the border were not specified, because the CSS spec is incorrect, but the rest of the spec is at least still there.

    Sure enough II. Went into kses.php and found line #1208:

    if ( preg_match( '%[\\(&]|/\*%', $css ) ) // remove any inline css containing \ ( & or comments
    return '';

    Removed the "(" making it:

    if ( preg_match( '%[\\&]|/\*%', $css ) ) // remove any inline css containing \ ( & or comments
    return '';

    and the post code remains untouched when the page refreshes.

    Now, HOW do we address this and, further, why has no one else reported this problem? Is my WPMu somehow different? Can we just eliminate the modifying/replacing of CSS styles altogether or can it be a user selectable option? I mean, AtD is a spelling, grammar syntax checker. It doesn't need to be messing with code that a reader never sees. That should be left up to WP core to sanitize.

12. rsmudge
    Member
    Posted 2 years ago #

    AtD's HTML editor proofreader transfers your HTML into a DOM so it can manipulate it. When it's done, it translates that DOM into a string and places that into the HTML editor.

    I think this can be eliminated by escaping the HTML when it's translated into a DOM element and unescaping it when send it to the server and placing it back into the HTML editor.

    Before going with this, I have to test it extensively to make sure that the escaped HTML does not interfere with my code finding errors sent back by the server and highlighting them.

    http://openatd.trac.wordpress.org/ticket/16

13. spherical
    Member
    Posted 2 years ago #

    Hi Raphael,

    That sounds like it would do it. Also seems that it might lessen the load on your servers if entire blocks of code can be excluded from eval and rewrite; only paying attention to stuff between ><.

    In the mean time, can you think of any risks involved in not stripping () from CSS tags? I'm just running scenarios on why the Dev Team would check for their presence if there wasn't a potential security hole. \ () & certainly are used in programming, so I'm figuring that there may be an injection possibility if they are allowed. Of course, if your above enhancement to AtD works out, I can restore the WP core syntax check to its original state and all is good.

    Thanks for looking into this. I really appreciate the functionality of AtD. It's caught a few grammatical structures that I didn't know were better written in other ways.

Topic Closed

This topic has been closed to new replies.