[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


Mikhail Pozdnyakov <mikhail.pozdnyakov at intel.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikhail.pozdnyakov at intel.co
                   |                            |m




--- Comment #11 from Mikhail Pozdnyakov <mikhail.pozdnyakov at intel.com>  2012-11-15 04:09:37 PST ---
(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 174331 [details] [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.

I would not put it as a rule to be done dogmatically every time, and introduce the check only in cases when we really have an expensive operation.

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