[Webkit-unassigned] [Bug 30555] Provide a way to get counter values with layoutTestContoller

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Shinichiro Hamaji <hamaji at chromium.org>  2009-10-22 02:08:54 PDT ---
Thanks for the detailed comments. The advises were really helpful!

> For our more modern tests, we do more than just avoid dumping the render tree.
> We also like do to targeted tests and use the script-tests system so we can
> test lots of things in a clear way in a single test. For those purposes, the
> most useful kind of feature would be one where we could get the counter text
> for a particular element in the document. We would want a JavaScript function
> to return that text given the element, not one that dumps all the counters in
> an entire document. That kind of feature would enable us to make thorough, easy
> to understand regression tests using the script-tests machinery and would be a
> lot more valuable than a dump feature.

I see. In my new patch, I've added layoutTestContoller.getCounterValueById,
instead of dumpCounterNode.

>     1) Take my comment above to heart and instead add a function that gives the
> counter value for just one node. The function would take either a Node* or an
> Element* and get the corresponding render object, then the counter return the
> text that would be rendered.

I like this approach. I have a few questions:

1. As inserted/removed DOM elements aren't layouted
   (element->renderer() is NULL) when the element is added, I needed
   to wait the layout using setTimeout before I obtain counter values
   by getCounterValueById. Is this unavoidable? I think calling
   layout() here is not good as same reason as externalRepresentation
   you mentioned. If there are no way to avoid this limitation,
   WebCore::getCounterValue should return a warning message like "this
   element is not layouted yet!" to relax potential confusion?

2. I'm not sure if the file I added functions are correct. I've added:

   - WebFrame(WebKitDebug).getCounterValue in WebKit/mac/Misc/WebCoreStatistics
   - WebCore::getCounterValue in WebCore/rendering/RenderTreeAsText

   If there are better places for them, please let me know and I'll
   move them.

> My other concern about this patch is that it only seems to take care of the Mac
> platform. When adding something like this, please do as many platforms as you
> can. The Windows platform used by my team here at Apple is almost always kept
> up to date when making a change like this, and I'd like to see at least that.

OK, I'll try working on other platforms as well. But, I don't want to update my
patch for many platforms several times. So, I'd like to work with the following
scheme:

1. Fix the design of the API using the patch for Mac.
2. Once Mac patch becomes good, I'll create patches for other platforms.
3. All of them get review+, then I check them in at once.

Is this OK?

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