[webkit-reviews] review denied: [Bug 16482] hook up ICU's encoding detector and add a boolean param to Settings and WebPreferences : [Attachment 23070] patch with some heuristics added to Eric's previous patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied 's request for review:
Bug 16482: hook up ICU's encoding detector and add a boolean param to Settings
and WebPreferences
https://bugs.webkit.org/show_bug.cgi?id=16482

Attachment 23070: patch with some heuristics  added to	Eric's previous patch 
https://bugs.webkit.org/attachment.cgi?id=23070&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
This patch looks good in general, however we need to split this ICU-specific
logic out into its own file, or we'll break non-ICU ports.  I would suggest
creating new files in platform/text named EncodingDectector.h
EncodingDectectorNone.cpp EncodingDetectorICU.cpp and adding your logic in the
appropriate places, and providing an empty stub in the None case.  qt/gtk
builds can use the None, and Mac/Win/Chromium can use the ICU version.	You can
also of course call the files whatever would be most correct, I'm not sure if
"TextEncodingDetector" makes more sense or not.

Otherwise the patch looks great.  There are a few style issues like foo_bar
should be fooBar for variable names.

Thanks for the patch!