Bug 17487 - HTMLInput mysteriously fails to work if ICU dat file is missing
Summary: HTMLInput mysteriously fails to work if ICU dat file is missing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P5 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-02-22 01:23 PST by Sriram Neelakandan
Modified: 2008-02-27 08:51 PST (History)
2 users (show)

See Also:


Attachments
Print an error if ICU fails to initialize (1.35 KB, patch)
2008-02-24 21:08 PST, Sriram Neelakandan
no flags Details | Formatted Diff | Diff
Patch from svn-create-patch (1.13 KB, patch)
2008-02-27 01:48 PST, Sriram Neelakandan
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sriram Neelakandan 2008-02-22 01:23:29 PST
HTML input box will not work if ICU dat file is not found.

This happens due to failure in ICU's Textbreakiterator

If an appropriate error message is displayed it will be useful for developers.
Comment 1 Mark Rowe (bdash) 2008-02-22 03:21:43 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.
Comment 2 Sriram Neelakandan 2008-02-24 21:08:29 PST
Created attachment 19336 [details]
Print an error if ICU fails to initialize
Comment 3 Sriram Neelakandan 2008-02-24 21:10:41 PST
(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)




Comment 4 Alexey Proskuryakov 2008-02-26 09:09:11 PST
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.
Comment 5 Mark Rowe (bdash) 2008-02-26 11:35:41 PST
Can you please also create the patch using svn-create-patch.  See <http://webkit.org/coding/contributing.html> for more information.
Comment 6 Sriram Neelakandan 2008-02-27 01:48:01 PST
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 7 Alexey Proskuryakov 2008-02-27 07:54:08 PST
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.
Comment 8 Alexey Proskuryakov 2008-02-27 08:51:52 PST
Committed in <http://trac.webkit.org/projects/webkit/changeset/30624>.