Bug 8910 – Various code cleanups in RenderBox

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Sam Weinig]

mostly deals with calcWidth()

[Sam Weinig]

Created attachment 8312 [details]
patch

cleanup and slight optimizations

[Darin Adler]

Comment on attachment 8312 [details]
patch

I'm not fond of adding parentheses around the equality comparisons in if statements. I don't like formatting where a continued if condition has the same indenting level as the code that will execute if the if is true. I typically indent the if condition an extra level to avoid this.

In headers, I like leaving out the parameter name if it's completely redundant with the type, so all those "WidthType widthType" should just be "WidthType".

+            int maxW =  calcWidthUsing(MaxWidth, containerWidth);

Two spaces here after the = sign, should just be one.

I don't understand why we'd set m_marginLeft and m_marginRight to 0 just before setting them inside the width.isAuto() case. Can those lines go inside the else?

The changes look good otherwise. I like the continued cleanup of calcWidthUsing. I'm tempted to say review+, but since this is all about style I think I'll say review- for the m_marginLeft/Right thing if for nowthing else.

[Sam Weinig]

Created attachment 8317 [details]
patch 2

updated to address darin's comments

[Darin Adler]

Comment on attachment 8317 [details]
patch 2

r=me