[Webkit-unassigned] [Bug 89751] Change RenderTable sections' iterations for removing anti-patterns and using helper functions.

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Arpita Bahuguna <arpitabahuguna at gmail.com>  2012-06-28 04:26:26 PST ---
(In reply to comment #6)
> (In reply to comment #4)
> 
> Thanks for the review Julien. Have incorporated the changes as suggested by you.
> 
> > (From update of attachment 148985 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=148985&action=review
> > 
> > > Source/WebCore/ChangeLog:3
> > > +        Refactoring RenderTable.cpp for code re-usability
> > 
> > This is a pretty bad name for a bug as it describes in no-way what you are doing.
> > 
> > I would rather rename it to something like (see below why):
> > * Change RenderTable sections' iterations to use the helper functions
> > * Unify RenderTable sections' iterations
> > 
> > > Source/WebCore/rendering/RenderTable.cpp:1000
> > > +    if (RenderTableSection* section = bottomSection()) {
> > 
> > This is definitely a good change. However if we are to use the helper more, I would just go ahead and kill a lot the following anti-pattern as part of this change:
> > 
> > for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
> >     if (child->isTableSection()) {
> >         ....
> >     }
> > }
> 
> This change can not be done for iterations in recalcSections() and appendColumn() since m_needsSectionRecalc would be set and thus any call to topSection() would fail in ASSERT(!needsSectionRecalc());

Also, am not really sure whether we should change the iteration in RenderTable::splitColumn().
It currently doesn't fail any of the existing testcases but there could probably be a scenario, similar to appendColumn(), wherein needsSectionRecalc could be set, in which case it would be inappropriate to iterate over RenderTableSections.

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