[Webkit-unassigned] [Bug 30505] Counters aren't updated when a new counter-increment is added

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Shinichiro Hamaji <hamaji at chromium.org>  2009-11-02 12:56:40 PDT ---
I'll describe this issue and why I chose this way.

Now, counter values are calculated when their originalText() are called.
originalText() calls counter() and it traverses the tree in ascending order to
update counter values. This approach works if there are no dynamic changes. 

However, if a new increment element (say [I]) is inserted before an
RenderCounter element (say [A]) and no other RenderCounter are inserted after
the increment element, the counter value of [A] won't be updated because
originalText() for [A] doesn't trigger counter() and it just uses the value of
counter node which are already constructed.

(Note that if another RenderCounter (say [B]) is added after [I], [B]'s
originalText() triggers counter(), notices [I] is added, creates a counter node
for [I], and updates all counter values including [A]'s)

I came up with two solutions for this:

- Insert counter nodes when counter elements are added and this addition will
update the values of descendants if needed. This is what my patch is doing.
- When a counter element is added, mark descendants of the added element as
dirty and make the counter values to be updated in the next call of
originalText(). If we can mark descendants as dirty, I think we can update the
values of counters when we mark. That's why I didn't choose this way.

Of course, there may be another better way which fixes this issue. If someone
kindly suggest better ways, I'm happy to fix my patch.

As for extra counter nodes, my patch doesn't create extra counter nodes
actually. The counter() function doesn't create new counter nodes if
corresponding counter node already exists.

> Further, what does isRooted have to do with any of this? It seems clearly wrong
> to be walking up the entire render tree looking for a RenderView in this code.

Ah, I think !isAnonymous() is much better here. What I want to do is

- for increment and reset, we create counter nodes when these nodes are added
- for content:counter(...), we create counter nodes when their originalText is
called(). Because :before and :after are not connected to the root node in
RenderObject::addChild(), we cannot create counter nodes for them with correct
values here. Deferring counter node creation for them are OK because 1. they
don't change the values of descendants as they aren't increment nor reset 2.
originalText() will be called for them and their values will be calculated
eventually.

I used isRooted() because of the second item, but I've noticed !isAnonymous()
is sufficient for this purpose so I changed to use it.

> Brace needs to be on the following line, not the same line.

Sorry for this mistake... I often forgot this rule (and forgot to use
check-webkit-style as well)

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