[webkit-reviews] review denied: [Bug 24114] ESMP(ECMAScript Mobile Profile) not supported : [Attachment 30648] Latest patch for ESMP to support casesensitive attribute of element

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 18 17:20:00 PDT 2009


Eric Seidel <eric at webkit.org> has denied yichao.yin
<yichao.yin at torchmobile.com.cn>'s request for review:
Bug 24114: ESMP(ECMAScript Mobile Profile) not supported
https://bugs.webkit.org/show_bug.cgi?id=24114

Attachment 30648: Latest patch for ESMP to support casesensitive attribute of
element
https://bugs.webkit.org/attachment.cgi?id=30648&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I think the case sentitive support can be enabled in Element always, w/o
guards.  We don't get much from the guards:
5 #if ENABLE(ESMP)
 56	PassRefPtr<Node> removeNamedItem(const String& name, ExceptionCode&
code, bool caseSensitive = false);
 57 #else
5458	 PassRefPtr<Node> removeNamedItem(const String& name, ExceptionCode&);
 59 #endif

Basically I would remove the guards on all of the internal plumbing.  The only
thing which needs guards are the JS bindings.

Why the copies of JSElment getAttribute and removeAttribute?  Those don't seem
needed.

r- for the excessive use of #ifdef guards and the code duplication.


More information about the webkit-reviews mailing list