[webkit-reviews] review denied: [Bug 12686] REGRESSION: Bloglines.com Feeds tab cannot expand folders in TOT : [Attachment 13306] patch for review

bugzilla-request-daemon at macosforge.org bugzilla-request-daemon at macosforge.org
Wed Feb 21 17:16:23 PST 2007


Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 12686: REGRESSION: Bloglines.com Feeds tab cannot expand folders in TOT
http://bugs.webkit.org/show_bug.cgi?id=12686

Attachment 13306: patch for review
http://bugs.webkit.org/attachment.cgi?id=13306&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I believe this patch is incorrect. The constructor objects should return true
for implementsHasInstance, but actual DOM element instances should not.

Thus I think the implementsHasInstance function should be in the constructor
class definition inside CodeGeneratorJS.pm. It can go right above the
implementsConstruct function.

To minimize the total amount of code inside CodeGeneratorJS.pm, we might want
to make a DOMConstructor base class to inherit from. That's not needed to fix
this bug, but I do see a reasonable amount of code that could be moved out of
the auto-generation script, including:

    1) the setPrototype call in the constructor
    2) the getValueProperty member function
    3) the implementsConstruct member function
    4) the implementsHasInstance member function

But I'm not sure it's worth it.

We use spaces for indenting, not tabs. The patch has a number of tabs in it.



More information about the webkit-reviews mailing list