FUDforum
Fast Uncompromising Discussions. FUDforum will get your users talking.

Home » FUDforum Development » Bug Reports » img bug introduced in most recent revision
Show: Today's Messages :: Polls :: Message Navigator
Switch to threaded view of this topic Create a new topic Submit Reply
img bug introduced in most recent revision [message #158587] Thu, 05 March 2009 06:21 Go to next message
Marticus
Messages: 272
Registered: June 2002
Karma: 1
Senior Member
When editing a post with images, the html never goes back to fud code for editing. I've tried modifying the post_proc.php.t file with no luck. Simply adding a " /" does not fix it.

[Updated on: Thu, 05 March 2009 06:48]

Report message to a moderator

Re: img bug introduced in most recent revision [message #158589 is a reply to message #158587] Thu, 05 March 2009 06:45 Go to previous messageGo to next message
Marticus
Messages: 272
Registered: June 2002
Karma: 1
Senior Member
This is resolved by falling back to previous revision of post_proc.php.t, it seems the addition of the quotes around the value for border= tag wasn't appreciated very much. I know we get in a hurry a lot, but we absolutely must be testing every change we make no matter how innocent they may seem.

Why do the quotes cause this problem? You can duplicate the problem right here if you want.

[Updated on: Thu, 05 March 2009 06:48]

Report message to a moderator

Re: img bug introduced in most recent revision [message #158592 is a reply to message #158589] Thu, 05 March 2009 10:51 Go to previous messageGo to next message
naudefj is currently offline  naudefj   South Africa
Messages: 3771
Registered: December 2004
Karma: 28
Senior Member
Administrator
Core Developer
Hi Marticus,

The bug-rate is extremely low given the amount of changes introduced. However, I would be keen to know how "we" (as you've rightfully stated) can test every feature of such a large project more effectively. Maybe someone need to write a test-suite for us?

Anyway, here is a patch (I'm sure you could've prepared it yourself). However, I'm not going to commit it before "we" haven't tested it properly.

Index: post_proc.inc.t
===================================================================
RCS file: /forum21/install/forum_data/src/post_proc.inc.t,v
retrieving revision 1.99
diff -u -r1.99 post_proc.inc.t
--- post_proc.inc.t     22 Feb 2009 00:00:37 -0000      1.99
+++ post_proc.inc.t     5 Mar 2009 10:49:33 -0000
@@ -588,11 +588,11 @@
        ),
        $fudml);

-       while (preg_match('!<img src="(.*?)" border="0" alt="\\1">!is', $fudml)) {
-               $fudml = preg_replace('!<img src="(.*?)" border="0" alt="\\1">!is', '[img]\1[/img]', $fudml);
+       while (preg_match('!<img src="?(.*?)"? border="0" alt="\\1" ?/?>!is', $fudml)) {
+               $fudml = preg_replace('!<img src="?(.*?)"? border="0" alt="\\1" ?/?>!is', '[img]\1[/img]', $fudml);
        }
-       while (preg_match('!<img class="(r|l)" src="(.*?)" border="0" alt="\\2">!is', $fudml)) {
-               $fudml = preg_replace('!<img class="(r|l)" src="(.*?)" border="0" alt="\\2">!is', '[img\1]\2[/img\1]', $fudml);
+       while (preg_match('!<img class="(r|l)" src="(.*?)" border="?0"? alt="\\2" ?/?>!is', $fudml)) {
+               $fudml = preg_replace('!<img class="(r|l)" src="?(.*?)"? border="0" alt="\\2" ?/?>!is', '[img\1]\2[/img\1]', $fudml);
        }
        while (preg_match('!<a href="mailto:(.+?)" target="_blank">\\1</a>!is', $fudml)) {
                $fudml = preg_replace('!<a href="mailto:(.+?)" target="_blank">\\1</a>!is', '[email]\1[/email]', $fudml);
@@ -602,10 +602,10 @@
        }

        if (strpos($fudml, '<img src="') !== false) {
-               $fudml = preg_replace('!<img src="(.*?)" border="0" alt="(.*?)">!is', '[img=\1]\2[/img]', $fudml);
+               $fudml = preg_replace('!<img src="(.*?)" border="?0"? alt="(.*?)" ?/?>!is', '[img=\1]\2[/img]', $fudml);
        }
        if (strpos($fudml, '<img class="') !== false) {
-               $fudml = preg_replace('!<img class="(r|l)" src="(.*?)" border="0" alt="(.*?)">!is', '[img\1=\2]\3[/img\1]', $fudml);
+               $fudml = preg_replace('!<img class="(r|l)" src="(.*?)" border="?0"? alt="(.*?)" ?/?>!is', '[img\1=\2]\3[/img\1]', $fudml);
        }
        if (strpos($fudml, '<a href="mailto:') !== false) {
                $fudml = preg_replace('!<a href="mailto:(.+?)" target="_blank">(.+?)</a>!is', '[email=\1]\2[/email]', $fudml);


Best regards.

Frank
Re: img bug introduced in most recent revision [message #158622 is a reply to message #158592] Sun, 08 March 2009 19:18 Go to previous messageGo to next message
Marticus
Messages: 272
Registered: June 2002
Karma: 1
Senior Member
Thanks, Frank. I'm not really up on regular expressions. I notice that some of your " are now followed by ? but not all of them, and the ' />' is now ' ?/?>'. Am am testing it momentarily, but I am curious about the inconsistency of the "?

What does this do exactly?

Before I go on, Frank and everyone involved, I really wanted to point out that this project is amazing and you guys do so much for it and, while it may not be a thankless job, you certainly don't get paid for countless dedicated hours working on it.

I for one really appreciate all the work done here and am always happy when my thoughts help out even in the smallest way. That being said, the following is only to provide a few ideas that might help improve the methods used for maintaining the code base.

Also note, my comment that brought this up was not intended to be spiteful in any way. Curiosity is a significant part of my character archetype.

As for testing, I think waiting longer might before going live might help. I do understand that small bugs make it into the final code sometimes. More time for the public to play with RCs should yield better results. Not sure what the bell curve looks like on that one, but I know a week doesn't even come close to the climax Smile

I also know this was a special release to placate several concerned parties, but when the RC came out, they were already ecstatic. I don't think they would have minded waiting a little longer for a final release.

It might also be worth making a list of svn comments as things are committed. For my own smaller projects, I keep a text document with a compiled list of comments along with the file names affected. I add to this list every time I test and commit something. Then I give that to my beta testers and QA so they know what to pinpoint, even after I test it myself. Sometimes my tests aren't nearly as aggressive as theirs.

This may sound like a lot of work, but it takes me only a couple seconds to copy and paste and another 30 seconds to test a feature associated with the change before committing it.
Re: img bug introduced in most recent revision [message #158623 is a reply to message #158622] Sun, 08 March 2009 19:21 Go to previous messageGo to next message
Marticus
Messages: 272
Registered: June 2002
Karma: 1
Senior Member
It would also help for our usual RC testers to check svn as well to see the list of changes made since the last release. I am guilty of not doing this as well.
Re: img bug introduced in most recent revision [message #158624 is a reply to message #158623] Sun, 08 March 2009 19:52 Go to previous messageGo to next message
Marticus
Messages: 272
Registered: June 2002
Karma: 1
Senior Member
Okay, I tested it and it works.

Two concerns. What does the ? do in the cases of ' ?/?>' and '"?' in the new code? Secondly, there is a new behavior. I think I resolved it by adding "? in the remainder of the border elements.

Let me explain.

The first preg_match replaces the xhtml code with \1, rendering the following response when you click on edit.

[img]index.php?t=getfile&id=492&private=0[/img]


The second preg_match replaces the xhtml code with \2, which renders the following response when you click on edit.

[img=index.php?t=getfile&id=492&private=0]index.php?t=getfile&id=492&private=0[/img]


Two patches ago, the code was causing the first preg_match to pass. Now, the code is causing the second preg_match to pass. I do not know if this change in behavior was intended. With the following patch, it now passes the first preg_match as before.

--- post_proc.inc.t.orig 2009-03-08 13:29:18.000000000 -0600
+++ post_proc.inc.t 2009-03-08 13:46:07.000000000 -0600
@@ -633,11 +633,11 @@
    ),
    $fudml);

-   while (preg_match('!<img src="?(.*?)"? border="0" alt="\\1" ?/?>!is', $fudml)) {
-       $fudml = preg_replace('!<img src="?(.*?)"? border="0" alt="\\1" ?/?>!is', '[img]\1[/img]', $fudml);
+   while (preg_match('!<img src="?(.*?)"? border="?0"? alt="\\1" ?/?>!is', $fudml)) {
+       $fudml = preg_replace('!<img src="?(.*?)"? border="?0"? alt="\\1" ?/?>!is', '[img]\1[/img]', $fudml);
    }
    while (preg_match('!<img class="(r|l)" src="(.*?)" border="?0"? alt="\\2" ?/?>!is', $fudml)) {
-       $fudml = preg_replace('!<img class="(r|l)" src="?(.*?)"? border="0" alt="\\2" ?/?>!is', '[img\1]\2[/img\1]', $fudml);
+       $fudml = preg_replace('!<img class="(r|l)" src="?(.*?)"? border="?0"? alt="\\2" ?/?>!is', '[img\1]\2[/img\1]', $fudml);
    }
    while (preg_match('!<a href="mailto:(.+?)" target="_blank">\\1</a>!is', $fudml)) {
        $fudml = preg_replace('!<a href="mailto:(.+?)" target="_blank">\\1</a>!is', '[email]\1[/email]', $fudml);
Re: img bug introduced in most recent revision [message #158625 is a reply to message #158623] Sun, 08 March 2009 19:54 Go to previous messageGo to next message
naudefj is currently offline  naudefj   South Africa
Messages: 3771
Registered: December 2004
Karma: 28
Senior Member
Administrator
Core Developer
Hi Marticus,

The question marks indicate optional characters. It's required to correctly interpret the old HTML and new XHTML formats.

The 2.8.0 release was somewhat rushed. However, we still had more then 3 weeks of RC testing. Problem is that most users would rather wait for a stable release before doing anything. Anyway, as things stand now, we should probably start preparing a 2.8.1 release within the next couple of weeks.

Maybe we could adopt an even/odd release cycle similar to the Linux kernel where odd version numbers can have new features, but even versions will only have stability fixes.

Best regards.

Frank
Re: img bug introduced in most recent revision [message #158626 is a reply to message #158624] Sun, 08 March 2009 19:59 Go to previous messageGo to next message
Marticus
Messages: 272
Registered: June 2002
Karma: 1
Senior Member
To make it easier for everyone, combining your patch and my patch renders the following code:

--- post_proc.inc.t 2009-03-08 13:58:18.000000000 -0600
+++ post_proc.inc.t.new 2009-03-08 13:57:56.000000000 -0600
@@ -588,11 +588,11 @@
    ),
    $fudml);

-   while (preg_match('!<img src="(.*?)" border="0" alt="\\1">!is', $fudml)) {
-       $fudml = preg_replace('!<img src="(.*?)" border="0" alt="\\1">!is', '[img]\1[/img]', $fudml);
+   while (preg_match('!<img src="?(.*?)"? border="?0"? alt="\\1" ?/?>!is', $fudml)) {
+       $fudml = preg_replace('!<img src="?(.*?)"? border="?0"? alt="\\1" ?/?>!is', '[img]\1[/img]', $fudml);
    }
-   while (preg_match('!<img class="(r|l)" src="(.*?)" border="0" alt="\\2">!is', $fudml)) {
-       $fudml = preg_replace('!<img class="(r|l)" src="(.*?)" border="0" alt="\\2">!is', '[img\1]\2[/img\1]', $fudml);
+   while (preg_match('!<img class="(r|l)" src="(.*?)" border="?0"? alt="\\2" ?/?>!is', $fudml)) {
+       $fudml = preg_replace('!<img class="(r|l)" src="?(.*?)"? border="?0"? alt="\\2" ?/?>!is', '[img\1]\2[/img\1]', $fudml);
    }
    while (preg_match('!<a href="mailto:(.+?)" target="_blank">\\1</a>!is', $fudml)) {
        $fudml = preg_replace('!<a href="mailto:(.+?)" target="_blank">\\1</a>!is', '[email]\1[/email]', $fudml);
@@ -602,10 +602,10 @@
    }

    if (strpos($fudml, '<img src="') !== false) {
-       $fudml = preg_replace('!<img src="(.*?)" border="0" alt="(.*?)">!is', '[img=\1]\2[/img]', $fudml);
+       $fudml = preg_replace('!<img src="(.*?)" border="?0"? alt="(.*?)" ?/?>!is', '[img=\1]\2[/img]', $fudml);
    }
    if (strpos($fudml, '<img class="') !== false) {
-       $fudml = preg_replace('!<img class="(r|l)" src="(.*?)" border="0" alt="(.*?)">!is', '[img\1=\2]\3[/img\1]', $fudml);
+       $fudml = preg_replace('!<img class="(r|l)" src="(.*?)" border="?0"? alt="(.*?)" ?/?>!is', '[img\1=\2]\3[/img\1]', $fudml);
    }
    if (strpos($fudml, '<a href="mailto:') !== false) {
        $fudml = preg_replace('!<a href="mailto:(.+?)" target="_blank">(.+?)</a>!is', '[email=\1]\2[/email]', $fudml);
Re: img bug introduced in most recent revision [message #158627 is a reply to message #158625] Sun, 08 March 2009 20:01 Go to previous messageGo to next message
Marticus
Messages: 272
Registered: June 2002
Karma: 1
Senior Member
naudefj wrote on Sun, 08 March 2009 14:54

The question marks indicate optional characters. It's required to correctly interpret the old HTML and new XHTML formats.




Oh, duh. That makes sense as older img posts do not have the quotes. Gotcha. Thanks again.
Re: img bug introduced in most recent revision [message #158632 is a reply to message #158625] Sun, 08 March 2009 20:20 Go to previous messageGo to next message
Marticus
Messages: 272
Registered: June 2002
Karma: 1
Senior Member
naudefj wrote on Sun, 08 March 2009 14:54
Maybe we could adopt a even/odd release cycle similar to that of the Linux kernel where odd version numbers can have new features, but even versions will only have stability fixes.


I forgot to reply to this. It is an idea. It does seem overly complicated for this relatively small project. How would clients react to having a 2.9.x upstream and a 2.8.x upstream. It would be terribly confusing and make our support community lives more complicated as well. At which point would you consider going to a 2.11.x and 2.12.x? At which point would you consider going to a 3.x? I'm sure there are many other factors to consider, but I can't wrap my brain around that concept for such a small project. Of course, this is not my project so who am I to say? Smile
Re: img bug introduced in most recent revision [message #158638 is a reply to message #158632] Mon, 09 March 2009 06:35 Go to previous messageGo to next message
naudefj is currently offline  naudefj   South Africa
Messages: 3771
Registered: December 2004
Karma: 28
Senior Member
Administrator
Core Developer
It's just an idea: How do you balance the requirement for new exciting features vs. those with large established forums that want boring but ultra stable releases?

BTW: Thanks for the updated patch. I will retest and commit it later today.

Best regards.

Frank

Re: img bug introduced in most recent revision [message #158649 is a reply to message #158638] Mon, 09 March 2009 16:59 Go to previous message
naudefj is currently offline  naudefj   South Africa
Messages: 3771
Registered: December 2004
Karma: 28
Senior Member
Administrator
Core Developer
Patch was committed. See http://cvs.prohost.org/c/index.cgi/FUDforum/chngview?cn=11889
  Switch to threaded view of this topic Create a new topic Submit Reply
Previous Topic: deleting attachments causes attachment list problem
Next Topic: Users missing in Group Manager
Goto Forum:
  

-=] Back to Top [=-
[ Syndicate this forum (XML) ] [ RSS ]

Current Time: Fri Nov 22 01:26:20 GMT 2024

Total time taken to generate the page: 0.02792 seconds