[Webkit-unassigned] [Bug 23262] Dual lines in css2.1 layout tests do not match:

Mon Sep 7 23:20:58 PDT 2009

https://bugs.webkit.org/show_bug.cgi?id=23262

--- Comment #8 from Shinichiro Hamaji <hamaji at chromium.org>  2009-09-07 23:20:58 PDT ---
Thanks for the review!

(In reply to comment #5)
> (From update of attachment 35109 [details])
> The experts have been silent about your patch for over 2 weeks.  I'm not an
> expert here, but your patch looks non-harmful.
> 
> Some nits:
>  1 ï»¿<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN">
> 
> Why the BOM?
> 
> Why all the extra junk in the tests?
>  5   <link rel="help" href="http://www.w3.org/TR/CSS21/generate.html#counters">
>  6   <link rel="help"
> href="http://www.w3.org/TR/CSS21/generate.html#propdef-content">
>  7   <link rel="help" href="http://www.w3.org/TR/CSS21/syndata.html#counter">

Removed them.

> No need to run this off a timeout:
>  37  <body onload="setTimeout('run()', 0)">
> 
> Better to just run it from inline script by placing the script tag low enough
> in the body...  Then you don't need "notifyDone()" either.

This test checks if the counters work properly when some elements are
added/removed *after* the first rendering finishes. So, if we put the script in
<body> this only checks the first rendering. Counters are working properly for
the first rendering even before this change. That's why we need to use the
timeout for this test.

> Do these tests really need to dump the render tree?  Ideally tests should be
> js-only test first, if they can't be js-only, at least dump as text, and render
> tree dumping tests should be used as a last-resort.

I think we need to use dump render tree unfortunately. Because counter is a CSS
element, it isn't dumped in dumpAsText output and it cannot be obtained from
javascripts.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.