Bug 69061 - [EFL] Only set when the custom encoding is different from existing value
Summary: [EFL] Only set when the custom encoding is different from existing value
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-28 23:14 PDT by Gyuyoung Kim
Modified: 2011-10-04 21:55 PDT (History)
5 users (show)

See Also:

Attachments
Patch (1.84 KB, patch)
2011-09-28 23:37 PDT, Gyuyoung Kim 		no flags Details | Formatted Diff | Diff
Patch (1.74 KB, patch)
2011-09-29 16:48 PDT, Gyuyoung Kim 		no flags Details | Formatted Diff | Diff
Patch (1.66 KB, patch)
2011-10-03 16:52 PDT, Gyuyoung Kim 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) 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 Gyuyoung Kim 2011-09-28 23:14:55 PDT 
ewk_view_setting_encoding_custom_set() has set a new custom encoding unconditionally despite new custom encoding is same with existing value. This patch sets only new custom encoding when new custom encoding value is different from existing value.
Comment 1 Gyuyoung Kim 2011-09-28 23:37:23 PDT 
Created attachment 109124 [details]
Patch
Comment 2 Leandro Pereira 2011-09-29 06:18:52 PDT 
Comment on attachment 109124 [details]
Patch

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

Although this check could be made inside the embedder, it is harmless enough to be inside EWK. The change looks fine, just a minor nit in the ChangeLog entry.

> Source/WebKit/efl/ChangeLog:10
> +        ewk_view_setting_encoding_custom_set() has set a new custom encoding unconditionally
> +        despite new custom encoding is same with existing value. This patch sets only new
> +        custom encoding when new custom encoding value is different from existing value.

ewk_view_setting_encoding_custom_set() sets a new custom ... value, reloading the page even when not needed. This patch sets the custom encoding only when the new value differs from the current value.
Comment 3 Gyuyoung Kim 2011-09-29 16:48:16 PDT 
Created attachment 109221 [details]
Patch
Comment 4 Gyuyoung Kim 2011-09-29 16:49:52 PDT 
Leandro, thank you for your comment. I modify ChangeLog again.
Comment 5 Leandro Pereira 2011-09-30 06:50:01 PDT 
Comment on attachment 109221 [details]
Patch

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

> Source/WebKit/efl/ChangeLog:10
> +        ewk_view_setting_encoding_custom_set() has set a new custom encoding value, reloading
> +        the page even when not needed. This patch sets the custom encoding only when new value
> +        differ.

Sorry to nitpick, but I think something along the lines of "Only set the custom encoding value if it is different from the current value, to avoid reloading the page." would be more clear.
Comment 6 Gyuyoung Kim 2011-10-03 16:52:49 PDT 
Created attachment 109551 [details]
Patch
Comment 7 Gyuyoung Kim 2011-10-03 16:53:09 PDT 
I modify it again.
Comment 8 Leandro Pereira 2011-10-04 07:26:01 PDT 
Comment on attachment 109551 [details]
Patch

Informal r+.
Comment 9 Hajime Morrita 2011-10-04 20:49:44 PDT 
Comment on attachment 109551 [details]
Patch

Rubberstamped.
Comment 10 WebKit Review Bot 2011-10-04 21:55:15 PDT 
Comment on attachment 109551 [details]
Patch

Clearing flags on attachment: 109551

Committed r96676: <http://trac.webkit.org/changeset/96676>
Comment 11 WebKit Review Bot 2011-10-04 21:55:19 PDT 
All reviewed patches have been landed.  Closing bug.