[webkit-reviews] review denied: [Bug 30555] Provide a way to get counter values with layoutTestContoller : [Attachment 41912] Patch v7

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 26 17:57:54 PDT 2009


Darin Adler <darin at apple.com> has denied Shinichiro Hamaji
<hamaji at chromium.org>'s request for review:
Bug 30555: Provide a way to get counter values with layoutTestContoller
https://bugs.webkit.org/show_bug.cgi?id=30555

Attachment 41912: Patch v7
https://bugs.webkit.org/attachment.cgi?id=41912&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Looks good.

>   the purpose of this test. That's why we use timeout for this test. -->
>   <body onload="setTimeout('run()', 0)">

Now that we have a countervalueForElementById function you could make the test
work without a setTimeout.

The first set of calls to counterValueForElementById will force a layout and
compute the counter values. So then you will have a valid test of a subsequent
dynamic change. The test will write out the values both before and after the
dynamic change.

I don't see any need to have setTimeout any more.

> +    if (!element)
> +	   return NULL;

We use "0", not "NULL", for this in WebKit code.

I am not sure that you included changes to make bots stay green on non-Mac
platforms, either by skipping the tests or landing expected results that expect
the test to fail. If you did, I could say review+ and we could land this first
version.

For now, review- because of the two comments above and my uncertainty about
what you are doing for the other platforms at this time.


More information about the webkit-reviews mailing list