[Webkit-unassigned] [Bug 102054] [EFL][WK2] Add APIs to get/set whether scripts can open new windows.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=102054





--- Comment #8 from Christophe Dumez <christophe.dumez at intel.com>  2012-11-15 03:11:09 PST ---
(From update of attachment 174331)
View in context: https://bugs.webkit.org/attachment.cgi?id=174331&action=review

>>> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:323
>>> +    EINA_SAFETY_ON_NULL_RETURN_VAL(settings, false);
>> 
>> I think we don't need to set if enable value is same as current value.
>> 
>> For example, you may add this condition.
>> 
>> bool currentValue = settings->preferences()->javaScriptCanOpenWindowsAutomatically();
>> if (currentValue == enable)
>>     return;
> 
> I agree to your idea.
> But, other APIs to set enable value do not check above condition.
> Above condition must be added in this function?

I think such if check would make sense if the operation was expensive. However, these are settings and the behavior is usually to set a boolean flag in memory. In practice, adding the if check may make it slower for no good reason.

See for e.g.:

void Settings::setJavaScriptCanOpenWindowsAutomatically(bool javaScriptCanOpenWindowsAutomatically)
{
    m_javaScriptCanOpenWindowsAutomatically = javaScriptCanOpenWindowsAutomatically;
}

Would it really make sense to do if (m_javaScriptCanOpenWindowsAutomatically != javaScriptCanOpenWindowsAutomatically) m_javaScriptCanOpenWindowsAutomatically = javaScriptCanOpenWindowsAutomatically; ?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.