[Webkit-unassigned] [Bug 9767] WebPreference methods setAllowsAnimatedImages: and setAllowsAnimatedImageLooping: don't actually affect WebView behaviors

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 16 11:36:43 PST 2012


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





--- Comment #23 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2012-02-16 11:36:42 PST ---
(In reply to comment #15)
> (In reply to comment #13)
> > (From update of attachment 126910 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=126910&action=review
> > 
> > > Source/WebKit/chromium/public/WebSettings.h:70
> > > +    virtual void setAllowsAnimatedImages(bool) = 0;
> > 
> > nit: this should probably be "setAllowAnimatedImages" instead.  this matches
> > other similarly named functions.  i think you should make this change to
> > WebCore::Settings too.
> 
> Sorry, i just realized you said i should change it in WebCore::Settings as well, but there are other prefs that sort of use this style as well (e.g. loadsImagesAutomatically), and there was some existing boilerplate code in some places using allowsAnimatedImages, so that's why i went with it.
> 
> I can change the name everywhere anyway if you prefer (and perhaps split this patch into a webcore patch and a bunch of platform ones), let me know.

On WebCore::Settings, I see 3 functions that look like "setAllowFoo" and 0 functions that look like "setAllowsFoo".  I think you should go with "setAllowFoo" for that reason.  Keep the code consistent :-)

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



More information about the webkit-unassigned mailing list