[Webkit-unassigned] [Bug 11031] Another crazy counters bug

[bugzilla-daemon at webkit.org]

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





--- Comment #18 from Carol Szabo <carol.szabo at nokia.com>  2009-11-16 08:31:03 PST ---
(In reply to comment #17)
> It seems that Carol's patch would still need some more work. For example, Darin
> suggested me to invalidate subsequent counters instead of creating counter when
> a render object with counter directives is added, but this patch seems to do
> the latter way. So, even if we decide to ask Carol to fix this bug, I guess it
> would make sense to land Bug 30505 first so that reviewers don't need to say
> the same comments on Carol's patch. What do you think?
> 
> By the way, I'd like two more issues/requests in the patch:
> 
> > +    if (temp = node->parent())
> 
> I cannot compile your patch on my Mac due to this line. This line produces a
> GCC warning and WebKit is using -Werror on Mac.
> 
> I think it would be better to have more smaller tests so that we can easily
> distinguish the cause of failures in future. Specifically, I think this bug can
> be divided into 4 smaller issues which are related each other:
> 
> 1. dynamic addition of increment node
> 2. dynamic deletion of increment node
> 3. dynamic addition of reset node
> 4. dynamic deletion of reset node
> 
> Once these four issues are fixed, I think it's very easy to fix style change
> case. So, I would like to have four tests. I was planning to fix this bug with
> 4 steps, and Bug 30505 was the first step.

I will probably submit my final version of the patch by the end of tomorrow.
I in the solution that I am proposing, which handles all cases, the issues are
broken down differently on two different axis:
A. Cause of change:
1. Changing the DOM tree via adding/removing nodes or subtrees
->This is addressed by changes to RenderObject addChild/removeChild
2. Altering the style of an existing node
->This is addressed by changes to setStyle()

B. The effect of change on some counterNodes of the same hierarchy with the
counterNodes added/removed by either cause but different than the nodes
added/removed:
1. Change in values only
2. Change in location in the hierarchy.

While the first case is easily addressable without destroying and recreating
the affected counters, the second situation is not.

I have briefly looked at Shinichiro Hamaji's patch and I saw that he addresses
a very limited number of cases.
I will try to sync my patch with the latest entries. I would like to continue
working on this issue as I feel that I found a good and fast solution that
covers all cases.
I am glad to defend any decisions that I made and seem worthwhile.
I did not defend my position on recursive traversal of the counter tree not the
rendering tree as Darin thought, because I did not find it worthwhile to argue
that the counter tree is expected to be shallow ( cannot think of a 9 levels
counter hierarchy that would be readable).
I am open for suggestions for optimizations as long as the optimized code still
covers all cases.
I do not yet understand the concern about updateCounters.
I am usually available for discussions on #webkit IRC channel on freenonde.

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