Projects/Acid3: Difference between revisions

Revision as of 19:45, 19 August 2008

Acid3 Test Failures of KHTML

This is an overview of the remaing failures of KHTML on the Acid3 test.

Test No	Area	Diagnosis	Comment	passes in FF3	passes in FF3.1
04	HTML Parser	Parser bug: iframe missing text kid	discard_until = ID_IFRAME+ID_CLOSE_TAG --- added in http://lists.kde.org/?l=kde-commits&m=99906936412933&w=2.	No	Yes
13	DOM Range	Unhandled mutation		Yes	Yes
26/27	JS + DOM Memory management	Cycle breaking cleaning up too much		Yes, but slow	Yes, but slow
29	HTML Parser	Parser bug: table missing whitespace kid		Yes	Yes
35	CSS	getComputedStyle() on <head> (no renderer)		Yes	Yes
41	CSS	getComputedStyle() on something else rendererless		Yes	Yes
43			Checkboxes, value, attributes	Yes	Yes
44	????	????		Yes	Yes
48 (red linktest failed)	CSS	:visited doesn't match relative URL right	Performance critical, hot on things like Qt docs, and already slow	Yes	Yes
51	DOM2 Table	Stray row	Buggy test. Raised with Ian Hickson.	Yes	Yes
53	DOM2 Forms	Not managing form's element collection when not in document.		Yes	Yes
65/69	Part loading	onload events not emitted for many objects		Yes, after several attempts	65 slow, 69 fails due to network timeout
70-80		Can't run due to 65/69	SVG, SMIL + XHTML	No	No

Note: An upgrade to PCRE 7.7 is required to make tests 89 and 90 pass.

Random patch storage

These are not meant for commit, but more as a proof of analysis, and starting point for proper fix:

#16, red cat.

Ugly, but roughly correct.

Index: kio/slavebase.cpp
===================================================================
--- kio/slavebase.cpp	(revision 793314)
+++ kio/slavebase.cpp	(working copy)
@@ -527,6 +527,7 @@
 void SlaveBase::errorPage()
 {
     send( INF_ERROR_PAGE );
+    mOutgoingMetaData["__kio_error_page"] = "1";
 }
 
 static bool isSubCommand(int cmd)
@@ -554,6 +555,11 @@
       KIO_DATA << mOutgoingMetaData;
       send( INF_META_DATA, data );
     }
+
+    // re-send the error-page flag as well.
+    if (mOutgoingMetaData.contains("__kio_error_page"))
+	send( INF_ERROR_PAGE );
+    
     KIO_DATA << _type;
     send( INF_MIME_TYPE, data );
     while(true)

#4, iframe kids.

This one may be committable, actually

--- a/html/htmlparser.cpp
+++ b/html/htmlparser.cpp
@@ -874,7 +874,7 @@ NodeImpl *KHTMLParser::getElement(Token* t)
         // a bit a special case, since the frame is inlined...
     case ID_IFRAME:
         n = new HTMLIFrameElementImpl(document);
-        if (!t->flat) discard_until = ID_IFRAME+ID_CLOSE_TAG;
+        //if (!t->flat) discard_until = ID_IFRAME+ID_CLOSE_TAG;
         break;

 // form elements

#35/#41

This one just shows that the analysis is correct. It's incorrect (no inheritance), and inefficient.

--- a/css/css_renderstyledeclarationimpl.cpp
+++ b/css/css_renderstyledeclarationimpl.cpp
@@ -26,6 +26,7 @@

 #include "cssproperties.h"
 #include "cssvalues.h"
+#include "cssstyleselector.h"

 #include <dom/dom_exception.h>

@@ -376,14 +377,20 @@ CSSValueImpl *RenderStyleDeclarationImpl::getPropertyCSSValue( int propertyID )
     }

     RenderObject *renderer = m_node->renderer();
+    SharedPtr<RenderStyle> onDemandStyle;
     if (!renderer) {
         // Handle display:none at the very least.  By definition if we don't have a renderer
         // we are considered to have no display.
         if (propertyID == CSS_PROP_DISPLAY)
             return new CSSPrimitiveValueImpl(CSS_VAL_NONE);
-        return 0;
+
+       // Brutal brute force path. Wrong due to inheritance. hmm.
+       if (node->isElementNode())
+           onDemandStyle = node->getDocument()->styleSelector()->styleForElement(static_cast<ElementImpl*>(node));
+       else
+           return 0;
     }
-    RenderStyle *style = renderer->style();
+    RenderStyle *style = renderer ? renderer->style() : onDemandStyle.get();
     if (!style)
         return 0;

@@ Line 9: / Line 9: @@
 ! Diagnosis
 ! Comment
-! passes in FF3b5
+! passes in FF3
+! passes in FF3.1
 |-
-| 04 || HTML Parser || Parser bug: iframe missing text kid || discard_until = ID_IFRAME+ID_CLOSE_TAG --- added in http://lists.kde.org/?l=kde-commits&m=99906936412933&w=2. || No
+| 04 || HTML Parser || Parser bug: iframe missing text kid || discard_until = ID_IFRAME+ID_CLOSE_TAG --- added in http://lists.kde.org/?l=kde-commits&m=99906936412933&w=2. || No || Yes
 |-
-| 13 || DOM Range || Unhandled mutation || || Yes
+| 13 || DOM Range || Unhandled mutation || || Yes || Yes
 |-
-| 26/27 || JS + DOM Memory management || Cycle breaking cleaning up too much || || Yes, but slow
+| 26/27 || JS + DOM Memory management || Cycle breaking cleaning up too much || || Yes, but slow || Yes, but slow
 |-
-| 29 || HTML Parser || Parser bug: table missing whitespace kid || || Yes
+| 29 || HTML Parser || Parser bug: table missing whitespace kid || || Yes || Yes
 |-
-| 35 || CSS ||  getComputedStyle() on <head> (no renderer) || || Yes
+| 35 || CSS ||  getComputedStyle() on <head> (no renderer) || || Yes || Yes
 |-
-| 41 || CSS ||  getComputedStyle() on something else rendererless ||  || Yes
+| 41 || CSS ||  getComputedStyle() on something else rendererless ||  || Yes || Yes
 |-
-| 43 ||  ||  || Checkboxes, value, attributes || Yes
+| 43 ||  ||  || Checkboxes, value, attributes || Yes || Yes
 |-
-| 44 || ????|| ???? ||  || Yes
+| 44 || ????|| ???? ||  || Yes || Yes
 |-
-| 48 (red linktest failed) || CSS || :visited doesn't match relative URL right || Performance critical, hot on things like Qt docs, and already slow || Yes
+| 48 (red linktest failed) || CSS || :visited doesn't match relative URL right || Performance critical, hot on things like Qt docs, and already slow || Yes || Yes
 |-
-| 51 || DOM2 Table || Stray row || Buggy test. Raised with Ian Hickson. || Yes
+| 51 || DOM2 Table || Stray row || Buggy test. Raised with Ian Hickson. || Yes || Yes
 |-
-| 53 || DOM2 Forms || Not managing form's element collection when not in document. || || Yes
+| 53 || DOM2 Forms || Not managing form's element collection when not in document. || || Yes || Yes
 |-
-| 65/69 || Part loading || onload events not emitted for many objects ||  || Yes, after several attempts
+| 65/69 || Part loading || onload events not emitted for many objects ||  || Yes, after several attempts || 65 slow, 69 fails due to network timeout
 |-
-| 70-80 || || Can't run due to 65/69 || SVG, SMIL + XHTML || No
+| 70-80 || || Can't run due to 65/69 || SVG, SMIL + XHTML || No || No
 |}