Add setting APIs for default text encoding and a unit test.
Created attachment 170356 [details] Patch
Comment on attachment 170356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170356&action=review > Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:277 > + Why not return the encoding name from WKPrefercenCopyDefaultTextEncodingName()? > Source/WebKit2/UIProcess/API/efl/ewk_settings_private.h:41 > + mutable WKEinaSharedString defaultTextEncoding; Why do we need to add this variable?
Comment on attachment 170356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170356&action=review > Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:265 > + EINA_SAFETY_ON_NULL_RETURN_VAL(encoding, false);?
Comment on attachment 170356 [details] Patch Needs rebasing due to recent Ewk_Settings design changes.
Comment on attachment 170356 [details] Patch Clearing the r? flag meanwhile.
Comment on attachment 170356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170356&action=review > Source/WebKit2/UIProcess/API/efl/ewk_settings.h:320 > +EAPI const char* ewk_settings_default_encoding_get(const Ewk_Settings *settings); Nit : wrong * place.
Created attachment 180964 [details] Patch
Comment on attachment 170356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170356&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:265 >> + > > EINA_SAFETY_ON_NULL_RETURN_VAL(encoding, false);? I added EINA_SAFETY_ON_NULL_RETURN_VAL(encoding, false). >> Source/WebKit2/UIProcess/API/efl/ewk_settings.h:320 >> +EAPI const char* ewk_settings_default_encoding_get(const Ewk_Settings *settings); > > Nit : wrong * place. I changed * place to "const char *ewk_settings_default_encoding_get". >> Source/WebKit2/UIProcess/API/efl/ewk_settings_private.h:41 >> + mutable WKEinaSharedString defaultTextEncoding; > > Why do we need to add this variable? Because memory for returned default encoding name is released after ewk_settings_default_encoding_get() is called, So, defaultTextEncoding variable is needed to keep returned default encoding name.
Comment on attachment 180964 [details] Patch LGTM.
Comment on attachment 180964 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180964&action=review > Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:59 > + if (defaultTextEncoding.isEmpty()) I wonder what meaning this condition has. Because, isEmpty() just checks if String has a character at least or null. Look at isEmpty() function. bool isEmpty() const { return !m_impl || !m_impl->length(); } However, I think "preferences()->defaultTextEncodingName()" won't be null. So, it seems to me that this condition isn't needed, doesn't it ? Is below enough ? m_defaultTextEncoding = preferences()->defaultTextEncodingName().utf8().data();
Comment on attachment 180964 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180964&action=review > Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:62 > + m_defaultTextEncoding = defaultTextEncoding.utf8().data(); Seems saved 'm_defaultTextEncoding' isn't used elsewhere. Then, can we remove it?
Comment on attachment 180964 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180964&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:62 >> + m_defaultTextEncoding = defaultTextEncoding.utf8().data(); This is incorrect. The char buffer is invalid pas that line. You need to hold on the CString if you want to keep a character buffer alive.
Comment on attachment 180964 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180964&action=review >>> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:62 >>> + m_defaultTextEncoding = defaultTextEncoding.utf8().data(); >> >> Seems saved 'm_defaultTextEncoding' isn't used elsewhere. >> Then, can we remove it? > > This is incorrect. The char buffer is invalid pas that line. > You need to hold on the CString if you want to keep a character buffer alive. m_defaulltTextEncoding is not a char* but a WKEinaSharedString. The string is actually copied so the code is correct. This is bit of a particularity of EFL port since our APIs returns EINA Stringshared strings.
Already fixed at Bug 126736 *** This bug has been marked as a duplicate of bug 126736 ***