Summary: | [EFL][WK2] defaultTextEncodingName should be stored as a member variable of EwkSettings | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jinwoo Song <jinwoo7.song> | ||||
Component: | WebKit EFL | Assignee: | Jinwoo Song <jinwoo7.song> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bunhere, cdumez, commit-queue, gyuyoung.kim, gyuyoung.kim, lucas.de.marchi, ryuan.choi | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Jinwoo Song
2014-01-22 02:52:36 PST
Created attachment 221849 [details]
Patch
Looks fine to me. Comment on attachment 221849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221849&action=review > Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:353 > +void EwkSettings::setDefaultTextEncodingName(const char* name) Nit, we normally added methods above the ewk APIs. Comment on attachment 221849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221849&action=review > Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:188 > + settings->setDefaultTextEncodingName(encoding); IIRC, we have tried to use C-API in ewk directly. For example, ewk_view.cpp, ewk_context_menu.cpp and so on. Can't we adjust it to ewk_settings as well ? Comment on attachment 221849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221849&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:188 >> + settings->setDefaultTextEncodingName(encoding); > > IIRC, we have tried to use C-API in ewk directly. For example, ewk_view.cpp, ewk_context_menu.cpp and so on. Can't we adjust it to ewk_settings as well ? There is a previous bug, https://bugs.webkit.org/show_bug.cgi?id=107820. I maybe follow up the bug after this one. How do you think about this? (In reply to comment #5) > (From update of attachment 221849 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221849&action=review > > >> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:188 > >> + settings->setDefaultTextEncodingName(encoding); > > > > IIRC, we have tried to use C-API in ewk directly. For example, ewk_view.cpp, ewk_context_menu.cpp and so on. Can't we adjust it to ewk_settings as well ? > > There is a previous bug, https://bugs.webkit.org/show_bug.cgi?id=107820. I maybe follow up the bug after this one. > How do you think about this? I see. Let's update the bug after landing this. Comment on attachment 221849 [details] Patch Clearing flags on attachment: 221849 Committed r162584: <http://trac.webkit.org/changeset/162584> All reviewed patches have been landed. Closing bug. |