[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 #9 from Gyuyoung Kim <gyuyoung.kim at samsung.com>  2012-11-15 03:47:20 PST ---
(In reply to comment #8)
> (From update of attachment 174331 [details])
> 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; ?

When we add this check, I also think there is no big performance gain because this is not expensive operation. In addition, to add this condition is not big operation. However, I think it doesn't generally make sense we set same value again. If the settings operation become a expensive stuff, I think this checking will prevent to waste our resource in future.

Honestly, now there is no gain by this checking. But, if we add additional operation in ewk_settings_xxx function as below,

Eina_Bool ewk_settings_encoding_detector_enabled_set(Ewk_Settings* settings, Eina_Bool enable)
{
    EINA_SAFETY_ON_NULL_RETURN_VAL(settings, false);

    settings->preferences()->setUsesEncodingDetector(enable);

    // To do something.

    return true;
}

In this case, we have to add this checking. But, I think it would be not bad if we add this now in order to prevent to do unnecessary operations in future.

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