RESOLVED FIXED 102867
[EFL][WK2] Remove setting word from ewk_view_setting_encoding_custom_XXX APIs 
https://bugs.webkit.org/show_bug.cgi?id=102867
Summary [EFL][WK2] Remove setting word from ewk_view_setting_encoding_custom_XXX APIs
Jongseok Yang
Reported 2012-11-20 20:38:00 PST
ewk_view_setting_encoding_custom_XXX APIs might be misunderstanded as the "setting" word because ewk_view_setting_encoding_custom_set triggers the "reload" operation. And ewk_view_setting_XXX is not correct because there is ewk_settings object for settings. The setting word from ewk_view_setting_encoding_custom_XXX APIs should be removed.
Attachments
Patch (6.31 KB, patch)
2012-11-20 20:40 PST, Jongseok Yang 		no flags
DetailsFormatted DiffDiff
Patch (6.31 KB, patch)
2012-11-21 02:18 PST, Jongseok Yang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jongseok Yang
Comment 1 2012-11-20 20:40:52 PST
Created attachment 175332 [details] Patch
Raphael Kubo da Costa (:rakuco)
Comment 2 2012-11-21 01:34:43 PST
Comment on attachment 175332 [details] Patch This looks like a nice improvement; I suggest a few additional ones: o Please mention in the _set() documentation that the page is going to be reloaded. o "encoding_custom" still sounds a bit weird. How about "ewk_view_custom_encoding_{get,set}" or "ewk_view_custom_text_encoding_{get,set}"?
Jongseok Yang
Comment 3 2012-11-21 01:48:43 PST
(In reply to comment #2) > (From update of attachment 175332 [details]) > This looks like a nice improvement; I suggest a few additional ones: > o Please mention in the _set() documentation that the page is going to be reloaded. > o "encoding_custom" still sounds a bit weird. How about "ewk_view_custom_encoding_{get,set}" or "ewk_view_custom_text_encoding_{get,set}"? Thanks for your review. I'll upload new patch.
Jongseok Yang
Comment 4 2012-11-21 01:58:16 PST
(In reply to comment #2) > (From update of attachment 175332 [details]) > This looks like a nice improvement; I suggest a few additional ones: > o Please mention in the _set() documentation that the page is going to be reloaded. I checked that it was already mentioned like the below. * Sets the custom character encoding and reloads the page. > o "encoding_custom" still sounds a bit weird. How about "ewk_view_custom_encoding_{get,set}" or "ewk_view_custom_text_encoding_{get,set}"? I'll change that.
Jongseok Yang
Comment 5 2012-11-21 02:18:38 PST
Created attachment 175386 [details] Patch
Raphael Kubo da Costa (:rakuco)
Comment 6 2012-11-21 02:31:34 PST
Comment on attachment 175386 [details] Patch LGTM.
WebKit Review Bot
Comment 7 2012-11-21 02:56:21 PST
Comment on attachment 175386 [details] Patch Clearing flags on attachment: 175386 Committed r135372: <http://trac.webkit.org/changeset/135372>
WebKit Review Bot
Comment 8 2012-11-21 02:56:26 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.