[webkit-reviews] review denied: [Bug 12159] Most of CSSComputedStyleDeclaration, cssparser.cpp, cssstyleselector.cpp should be autogenerated : [Attachment 16667] Move enum mapping into a separate file

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 15 07:43:53 PDT 2007


Darin Adler <darin at apple.com> has denied Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 12159: Most of CSSComputedStyleDeclaration, cssparser.cpp,
cssstyleselector.cpp should be autogenerated
http://bugs.webkit.org/show_bug.cgi?id=12159

Attachment 16667: Move enum mapping into a separate file
http://bugs.webkit.org/attachment.cgi?id=16667&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+	     ASSERT(boxAlign != BJUSTIFY);
+	     if (boxAlign == BJUSTIFY)
+		 return 0;

+	     ASSERT(pageBreak != PBALWAYS);
+	     if (pageBreak == PBALWAYS)
+		 return 0;

The checking here seems unnecessary. I think we can just omit it.

Do we have test cases that cover all the different length types and property
values?

-	 CSSPrimitiveValue* val = new CSSPrimitiveValue(pair);
+	 CSSPrimitiveValue* val = new
CSSPrimitiveValue(PassRefPtr<Pair>(pair));

I think that needing to do this is quite unfortunate. Because of this, I'm not
happy with the template conversion technique. Can we switch to a simpler
technique that doesn't involve templates?

We could just put all the enums into a separate header file and use plain old
overloading.

I otherwise think this is cool, and a great direction.


More information about the webkit-reviews mailing list