Review Request: Add Javascript policy related settings to newly added HtmlSettingsInterface class

[Dawit Alemayehu]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104179/

Review request for kdelibs and David Faure. By Dawit Alemayehu. Description The attached patch adds several JavaScript policy and setting related enums and three convenience functions to KParts::HtmlSettingsInterface so that they can be shared by all browser engines (kwebkitpart and khtml) and the Konqueror html kcm. Currently these enums and convenience functions are duplicated in the two browser engines. The Konqueror html kcm simply uses the one from the khtml browser engine because it publicly exports those values. Unfortunately that means the html kcm has to directly link against the khtml engine resulting in it being loaded into memory when the Konqueror configuration dialog is launched even regardless of what browser engine is used as the default. This patch is the first step in removing that dependency so that khtml is not loaded when webkit is used as the default browser engine. Diffs khtml/khtml_ext.cpp (3a1da4a) kparts/htmlextension.h (b91f203) kparts/htmlextension.cpp (761c42c) View Diff

[David Faure]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104179/

Ship it!

Looks good, just one question.

kparts/htmlextension.cpp (Diff revision 1)
		const char* HtmlSettingsInterface::javascriptAdviceToText(HtmlSettingsInterface::JavaScriptAdvice advice)
		114	return I18N_NOOP("Accept");

Any idea why this uses I18N_NOOP? Is this so that the untranslated string gets used in config files, but the GUI code can call i18n() on it for display purposes?
If this is indeed used like that, please document it, otherwise remove I18N_NOOP.

- David

On March 10th, 2012, 3:39 p.m., Dawit Alemayehu wrote:

Review request for kdelibs and David Faure. By Dawit Alemayehu.

Updated March 10, 2012, 3:39 p.m.

[Dawit Alemayehu]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104179/

On March 11th, 2012, 8:52 a.m., David Faure wrote:

kparts/htmlextension.cpp (Diff revision 1)
		const char* HtmlSettingsInterface::javascriptAdviceToText(HtmlSettingsInterface::JavaScriptAdvice advice)
		114	return I18N_NOOP("Accept");

Any idea why this uses I18N_NOOP? Is this so that the untranslated string gets used in config files, but the GUI code can call i18n() on it for display purposes?
If this is indeed used like that, please document it, otherwise remove I18N_NOOP.

Yes. That seems to be the purpose of it. It was literally copied from khtml just as it appears, but checking the code reveals that whatever is returned by that function is indeed being password to i18n. I will document it as such.

- Dawit

[Commit Hook]

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104179/

This review has been submitted with commit 212441d20f21abe2d751ad4283e7035f330d4f23 by Dawit Alemayehu to branch KDE/4.8.

- Commit

On March 10th, 2012, 3:39 p.m., Dawit Alemayehu wrote:

Review request for kdelibs and David Faure. By Dawit Alemayehu.

Updated March 10, 2012, 3:39 p.m. Description