Bug 116790

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