[webkit-reviews] review denied: [Bug 41510] Add ignoreWhitespace option at base64Decode() : [Attachment 60348] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 2 12:02:45 PDT 2010


Alexey Proskuryakov <ap at webkit.org> has denied Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 41510: Add ignoreWhitespace option at base64Decode()
https://bugs.webkit.org/show_bug.cgi?id=41510

Attachment 60348: Patch
https://bugs.webkit.org/attachment.cgi?id=60348&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> What is the best way to do a LayoutTest for the change in Page.cpp? Example?

It would help if ChangeLog explained the change.

There is a number of calls for user style sheet manipulation in
layoutTestController, such as addUserStyleSheet, clearPersistentUserStyleSheet,
setPersistentUserStyleSheetLocation, setUserStyleSheetEnabled,
setUserStyleSheetLocation.

+    if (!base64Decode(in, out, false)) {

The person reading this code will have no idea about what "false" means. Please
use a named enum constant instead.

r- for lack of testing and ChangeLog explanation. I didn't check the actual
math.

See also: loosely related bug 23566.


More information about the webkit-reviews mailing list