Bug 17082

Summary: Acid3 bug: CSS objects in Webkit are not "live"
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit
Priority: P2 Keywords: HasReduction
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
URL: http://acid3.acidtests.org/
Bug Depends on:    
Bug Blocks: 17064    
Attachments:
Description Flags
minimal test case
none
Make the rules list live for stylesheets. oliver: review+

Description Eric Seidel (no email) 2008-01-29 15:07:08 PST
WebKit fails Acid3's modification of style blocks text node test

This will require some reduction.  It might be a bug in the test.

Test 73: Undefined value

    function () {
      // test 73: dynamic modification of <style> blocks' text nodes, from Jonas Sicking and Garret Smith
      var doc = kungFuDeathGrip.childNodes[3].contentDocument;
      assert(doc, "missing document for test");
      assert(doc.images[0], "prerequisite failed: no image");
      assertEquals(doc.images[0].height, 10, "prerequisite failed: style didn't affect image");
      assertEquals(doc.styleSheets[0].href, null, "internal stylesheet had a URI");
      doc.styleSheets[0].ownerNode.firstChild.data = "img { height: 20px; }";
      assertEquals(doc.images[0].height, 20, "change failed to take effect");
      doc.styleSheets[0].ownerNode.appendChild(doc.createTextNode("img { height: 30px; }"));
      assertEquals(doc.images[0].height, 30, "append failed to take effect");
      var rules = doc.styleSheets.cssRules; // "All CSS objects in the DOM are "live"" says section 2.1, Overview of the DOM Level 2 CSS Interfaces
      doc.styleSheets[0].insertRule("img { height: 40px; }", 2);
      assertEquals(doc.images[0].height, 40, "insertRule failed to take effect");
      assertEquals(doc.styleSheets.cssRules.length, rules.length, "cssRules isn't live");
      return 5;
    },
Comment 1 Eric Seidel (no email) 2008-01-30 01:17:57 PST
We'll need to build this into a nice stand-alone test case.
Comment 2 Robert Blaut 2008-02-04 05:22:53 PST
Created attachment 18905 [details]
minimal test case
Comment 3 Robert Blaut 2008-02-04 05:33:53 PST
The acid3 test 73 was changed a little:

 function () {
      // test 73: dynamic modification of <style> blocks' text nodes, from Jonas Sicking and Garret Smith
      var doc = kungFuDeathGrip.childNodes[3].contentDocument;
      assert(doc, "missing document for test");
      assert(doc.images[0], "prerequisite failed: no image");
      assertEquals(doc.images[0].height, 10, "prerequisite failed: style didn't affect image");
      doc.styleSheets[0].ownerNode.firstChild.data = "img { height: 20px; }";
      assertEquals(doc.images[0].height, 20, "change failed to take effect");
      doc.styleSheets[0].ownerNode.appendChild(doc.createTextNode("img { height: 30px; }"));
      assertEquals(doc.images[0].height, 30, "append failed to take effect");
      var rules = doc.styleSheets[0].cssRules; // "All CSS objects in the DOM are "live"" says section 2.1, Overview of the DOM Level 2 CSS Interfaces
      doc.styleSheets[0].insertRule("img { height: 40px; }", 2);
      assertEquals(doc.images[0].height, 40, "insertRule failed to take effect");
      assertEquals(doc.styleSheets[0].cssRules.length, 3, "count of rules is wrong");
      assertEquals(rules.length, 3, "cssRules isn't live");
      // while we're at it, check some other things on doc.styleSheets:
      assert(doc.styleSheets[0].href === null, "internal stylesheet had a URI: " + doc.styleSheets[0].href);
      assert(document.styleSheets[0].href === null, "internal acid3 stylesheet had a URI: " + document.styleSheets[0].href);
      return 5;
    },
Comment 4 Dave Hyatt 2008-02-10 02:54:56 PST
I'm working on this one.  Nearly have a fix ready.

Comment 5 Dave Hyatt 2008-02-10 03:02:11 PST
Created attachment 19034 [details]
Make the rules list live for stylesheets.

This code is just a huge mess.  This is the minimal surgery that I had to do to get this working (and should perform better).  The old code copied all of the rules into a separate list.   The new code will not do this if a StyleList is available, unless the IE rules() extension is being used (and charset rules have to be omitted).

In the longer term, this class could be fixed to just always reference StyleLists and to never have its own internal list.  It would have to operate a bit more like a collection in that case (knowing to skip charset rules if created as an IE rules() extension list).
Comment 6 Oliver Hunt 2008-02-10 13:30:10 PST
Comment on attachment 19034 [details]
Make the rules list live for stylesheets.

Testcases amd changelogs are your friend :p
Comment 7 Dave Hyatt 2008-02-10 14:33:40 PST
Fixed in r30130.