Projects/Acid3: Difference between revisions

Revision as of 23:08, 5 April 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 FF3b5
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
13	DOM Range	Unhandled mutation		Yes
18	????	????		Yes
Red image(16)	Part loading		Not doing fall-back content in the general delayed mimetype-resolution processObjectRequest path. The fallback in RenderImage would be sufficient, but fails because INF_ERROR_PAGE isn't re-sent with the mimetype when resuming a held job	Yes
26/27	JS + DOM Memory management	Cycle breaking cleaning up too much		Yes, but slow
29	HTML Parser	Parser bug: table missing whitespace kid		Yes
35	CSS	getComputedStyle() on <head> (no renderer)		Yes
38	CSS	Lack of restyle	"adding text to a text node didn't make the element non-:empty"	No
41	CSS	getComputedStyle() on something else rendererless		Yes
43			Checkboxes, value, attributes	Yes
44	????	????		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
51	DOM2 Table	Stray row	Buggy test. Raised with Ian Hickson.	Yes
53	DOM2 Forms	Not managing form's element collection when not in document.		Yes
65/69	Part loading	onload events not emitted for many objects		Yes, after several attempts
70-80		Can't run due to 65/69	SVG, SMIL + XHTML	No
89	JS RegExp	Lacking syntax check	Perl-centric PCRE behavior. Author promised JS-mode.	Yes
90	JS RegExp	References	Perl-centric PCRE behavior. Author promised JS-mode. (was it ever considered to use Boost.Regex engine instead ? it claims to be using strict ECMA syntax as per http://boost.org/doc/libs/1_35_0/libs/regex/doc/html/boost_regex/ref/syntax_option_type/syntax_option_type_perl.html Update: I tried to build a test application with Boost.Regex 1.33 and it had the same issues than PCRE: /a[])]/ compiled and /(\3)(\1)(a)/ went back as "Invalid Back Reference". -gg Seems that despite the name, it's not really ES compatible --- if the docs are right, it doesn't support \u.	Yes
98	XHTML	createDocument()	Missing view. See proposed patch	No

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;