Summary: | HTMLInput mysteriously fails to work if ICU dat file is missing | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sriram Neelakandan <sriram.neelakandan> | ||||||
Component: | Text | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Enhancement | CC: | ap, mrowe | ||||||
Priority: | P5 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Linux | ||||||||
Attachments: |
|
Description
Sriram Neelakandan
2008-02-22 01:23:29 PST
I don't see how this is a valid bug. If your ICU installation is broken and you are using a WebKit port that relies on ICU then many things will not work. Created attachment 19336 [details]
Print an error if ICU fails to initialize
(In reply to comment #1) > I don't see how this is a valid bug. If your ICU installation is broken and > you are using a WebKit port that relies on ICU then many things will not work. > I use GTK port. Its not that many things don't work. Every thing works but for HTMLTextInput. I already have sent a patch for the same. It just prints a message to indicate failure. Can u review it ? Please see this link http://lists.webkit.org/pipermail/webkit-dev/2008-February/003426.html After taking care of Coding Guidelines as mentioned by David, I have attached a modified version of the same patch to this bug (http://bugs.webkit.org/attachment.cgi?id=19336&action=view) I think that adding logging for this case is a very good idea. I also had a occasion when debugging an issue caused by a missing (actually, non-matching) ICU dat file took longer than it should have taken. We normally use LOG_ERROR macro in cases like this, not fprintf(stderr, ...) - could you please update the patch to use it? In the future, you can mark a patch for review by clicking "Edit" link - this guarantees that it won't get lost. Can you please also create the patch using svn-create-patch. See <http://webkit.org/coding/contributing.html> for more information. Created attachment 19399 [details]
Patch from svn-create-patch
I have corrected the patch as per coding guidelines.
also used LOG_ERROR instead of fprintf.
Also the patch has been taken using svn-create-patch
Comment on attachment 19399 [details]
Patch from svn-create-patch
There are still some issues with this patch, but I'm going to say r=me and fix those when landing.
1. There is no ChangeLog.
2. "+#include <stdio.h>" is no longer needed without fprintf.
3.
+ if (!createdIterator)
+ {
We put opening brace on the same line as "if".
4. Instead of a combination of LOG_ERROR and ASSERT, one can use ASSERT_WITH_MESSAGE.
Committed in <http://trac.webkit.org/projects/webkit/changeset/30624>. |