Bug 98726 - [EFL][EWebLauncher] Add encoding detector option.
Summary: [EFL][EWebLauncher] Add encoding detector option.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Kangil Han
URL:
Keywords:
Depends on: 61744
Blocks:
  Show dependency tree / graph
 
Reported: 2012-10-08 23:36 PDT by Kangil Han
Modified: 2012-10-15 10:06 PDT (History)
7 users (show)

See Also:

Attachments
patch (2.90 KB, patch)
2012-10-08 23:43 PDT, Kangil Han 		gyuyoung.kim: review+
Details | Formatted Diff | Diff
patch (2.89 KB, patch)
2012-10-09 04:49 PDT, Kangil Han 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kangil Han 2012-10-08 23:36:49 PDT 
We need this to test WebCore's encoding detector functionality.
Comment 1 Kangil Han 2012-10-08 23:43:44 PDT 
Created attachment 167698 [details]
patch
Comment 2 Ryuan Choi 2012-10-09 01:08:17 PDT 
(In reply to comment #1)
> Created an attachment (id=167698) [details]
> patch

LGTM
Comment 3 Gyuyoung Kim 2012-10-09 01:21:48 PDT 
Comment on attachment 167698 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167698&action=review

> Tools/EWebLauncher/main.c:151
> +    Eina_Bool enableEncodingDetector;

Alphabetical order ?

> Tools/EWebLauncher/main.c:917
> +    userArgs->enableEncodingDetector = EINA_FALSE;

ditto ?

> Tools/EWebLauncher/main.c:931
> +        ECORE_GETOPT_VALUE_BOOL(userArgs->enableEncodingDetector),

ditto ?
Comment 4 Kangil Han 2012-10-09 04:49:51 PDT 
Created attachment 167735 [details]
patch

Done~
Comment 5 Kenneth Rohde Christiansen 2012-10-09 04:51:22 PDT 
Comment on attachment 167735 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167735&action=review

> Tools/ChangeLog:10
> +
> +        Added an option to test WebCore's encoding detector functionality on EWebLauncher.
> +        With this patch, EWebLauncher would display text correctly even if web page wouldn't specify charset information.
> +

Why isnt this default? and what about WebKit2, isn't that our target?
Comment 6 Kangil Han 2012-10-09 04:57:22 PDT 
(In reply to comment #5)
> Why isnt this default?

Would you like to enable this functionality by default?

> and what about WebKit2, isn't that our target?

Yuni Jeong has prepared to create a patch for adding a setting API to enable encoding detector. I will follow up after her in MiniBrowser. :-)
Comment 7 Kenneth Rohde Christiansen 2012-10-09 04:59:22 PDT 
(In reply to comment #6)
> (In reply to comment #5)
> > Why isnt this default?
> 
> Would you like to enable this functionality by default?

Is there any reason not to? When don't you want this?
Comment 8 Ryuan Choi 2012-10-09 05:03:44 PDT 
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Why isnt this default?
> > 
> > Would you like to enable this functionality by default?
> 
> Is there any reason not to? When don't you want this?

In Bug 61744, it was disabled to pass dom/xhtml/level3/core/documentgetinputencoding01.xhtml
Comment 9 Kangil Han 2012-10-09 05:27:23 PDT 
(In reply to comment #8)
> In Bug 61744, it was disabled to pass dom/xhtml/level3/core/documentgetinputencoding01.xhtml

Thanks for this information. I didn't know that. :-)
Comment 10 Gyuyoung Kim 2012-10-09 05:29:32 PDT 
(In reply to comment #9)
> (In reply to comment #8)
> > In Bug 61744, it was disabled to pass dom/xhtml/level3/core/documentgetinputencoding01.xhtml
> 
> Thanks for this information. I didn't know that. :-)

Ok, let's decide to land this patch according to Bug 61744 direction.
Comment 11 Kangil Han 2012-10-11 23:28:49 PDT 
(In reply to comment #7)
> Is there any reason not to? When don't you want this?

Here is interesting result of layout test when enable/disable encoding auto detection by default. If we enable encoding auto detection, then will have additional 125 failed test cases.

FYI, this test has been done on my desktop, Ubuntu 12.04 64 bit debug build.

Given circumstance, let's use encoding detector as an optional functionality. :)

p.s. It seems other ports off this as default.
Comment 12 WebKit Review Bot 2012-10-15 10:06:35 PDT 
Comment on attachment 167735 [details]
patch

Clearing flags on attachment: 167735

Committed r131312: <http://trac.webkit.org/changeset/131312>
Comment 13 WebKit Review Bot 2012-10-15 10:06:40 PDT 
All reviewed patches have been landed.  Closing bug.