|
Re: img bug introduced in most recent revision [message #158589 is a reply to message #158587] |
Thu, 05 March 2009 06:45 |
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 |
|
naudefj
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 |
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
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 #158624 is a reply to message #158623] |
Sun, 08 March 2009 19:52 |
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 , 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 , 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 #158626 is a reply to message #158624] |
Sun, 08 March 2009 19:59 |
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 |
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 |
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?
|
|
|
|
|