Version: (using KDE KDE 3.5.0) Installed from: Unlisted Binary Package OS: Linux Browsing the site http://www.osnews.com/ I noticed that comments posted with Konqueror are not associated with a thread while when posted with Firefox they are. It turned out that Konqi doesn't send a hidden field that carries the parent post info. I stripped down the HTML as much as possible and the problem lies here (see also attached page): <tr> <td> <font> <p> <input type="hidden" name="ref" value="62637"> </font> </td> </tr> If I remove either the <font>, the </font> or the <p> tag the hidden field reappears in the request. --- Here is my version info (I'm using the experimental Debian 3.5 packages): > konqueror -v Qt: 3.3.5 KDE: 3.5 (RC1) Konqueror: 3.5
Created attachment 13572 [details] Stripped HTML page that reproduces the problem
Sorry, how do I configure something to use that test?
Created attachment 13573 [details] simpler testcase Here's a simpler testcase. Note that there's no table tag.
Ah yes, sorry, I had written a little php script that dumped the passed parameters. But I guess Niels' test case using method='get' will be sufficient, won't it?
Created attachment 13575 [details] Patch This patch fixes it by improving fall-back behaviour I am not sure it is the right way(TM). The real problem is that the htmlparser.cpp chokes on the broken HTML forms from osnews and forgets which form input element should refer to.
Yes, that patch fixes the problem both with my local test case and the real world example. Your speed is amazing. :) What will happen next?
SVN commit 497603 by carewolf: Fix OSNews replies BUG: 116790 M +5 -1 html_formimpl.cpp --- branches/KDE/3.5/kdelibs/khtml/html/html_formimpl.cpp #497602:497603 @@ -885,7 +885,11 @@ { if( p->id() == ID_FORM ) return static_cast<HTMLFormElementImpl *>(p); - p = p->parentNode(); + NodeImpl *s = p->previousSibling(); + if (!s) + p = p->parentNode(); + else + p = s; } #ifdef FORMS_DEBUG kdDebug( 6030 ) << "couldn't find form!" << endl;
SVN commit 517005 by ivor: Replace one form reattach hack with another, pending a better fix. BUG: 122968 BUG: 122273 CCBUG: 116790 M +1 -5 html_formimpl.cpp M +9 -0 htmlparser.cpp --- branches/KDE/3.5/kdelibs/khtml/html/html_formimpl.cpp #517004:517005 @@ -886,11 +886,7 @@ { if( p->id() == ID_FORM ) return static_cast<HTMLFormElementImpl *>(p); - NodeImpl *s = p->previousSibling(); - if (!s) - p = p->parentNode(); - else - p = s; + p = p->parentNode(); } #ifdef FORMS_DEBUG kdDebug( 6030 ) << "couldn't find form!" << endl; --- branches/KDE/3.5/kdelibs/khtml/html/htmlparser.cpp #517004:517005 @@ -1388,6 +1388,15 @@ blockElem->removeChild(currNode, exceptionCode); newNode->appendChild(currNode, exceptionCode); currNode = nextNode; + + // TODO - To be replaced. + // Re-register form elements with currently active form, step 1 will have removed them + if (form) + { + HTMLGenericFormElementImpl *e = static_cast<HTMLGenericFormElementImpl *>(currNode); + if (e) + form->registerFormElement(e); + } } // Step 4: Place |newNode| under |blockElem|. |blockElem| is still out of the document, so no
It seems that the original bug is back. :-( The test cases still work, but the reference is lost again when posting on osnews.com. Tested with: Qt: 3.3.6 KDE: 3.5.3 Konqueror: 3.5.3
The DOM is unchanged, and the test case show we send the hidden field... It almost sounds like OSNews PHP code is breaking us.
Created attachment 16693 [details] Test case This is the newest code from OSNews grapped today. Only thing changed is changing form-action from post to get. As demonstrated, Konqueror sends the ref value just fine. It must be OSNews that specifically ignores the 'ref' when UA is Konqueror.
Created attachment 16694 [details] Patch Ahh :) My original patch still works. Here's a modified version.
Unfortunately I no longer have any svn version of the 3.5 branch around so I cannot test the patch. Will it make it into the next point release?
SVN commit 563771 by carewolf: Fix for yet again broken replies in osnews.com. This implements the behavior on DOM level instead of at parsing. This also means that a reparsed DOM should work. BUG: 116790 M +5 -0 html_formimpl.cpp --- branches/KDE/3.5/kdelibs/khtml/html/html_formimpl.cpp #563770:563771 @@ -890,6 +890,11 @@ { if( p->id() == ID_FORM ) return static_cast<HTMLFormElementImpl *>(p); + if( p->parentNode() && p->parentNode()->id() == ID_TABLE && p->previousSibling() ) + { + p = p->previousSibling(); + continue; + } p = p->parentNode(); } #ifdef FORMS_DEBUG