On Mon, Sep 21, 2009 at 2:31 PM, Jeremy Orlow <[email protected]> wrote:
> I think we need to re-consider our practice of shipping beta/stable > browsers with experimental features hidden behind flags--at least when they > have any side-effects in JavaScript. An example of where this has bitten us > is http://code.google.com/p/chromium/issues/detail?id=22181 > > Although part of the problem is the way they coded things (since both > SessionStorage and LocalStorage use the Storage interface, > its existence doesn't imply SessionStorage is necessarily available), this > bug has pointed out a couple problems. 1) constructors are visible to > javascript even when the feature is totally disabled. > If it's behind a flag, it shouldn't have been exposed, right? On the surface, it sounds like this code was only partially hidden behind the flag? I think it would be a good idea to have a unit test which enumerates all symbols that we're exposing into JS. This should be a controlled list. If we had this unit test, would it have caught this exposure? Mike > 2) When an object (like the Storage interface) wraps a NULL it shows up as > null in JavaScript. Since returning NULL/0 is the standard thing to do when > the feature is disabled, this means that the functions return null when > disabled at run time and undefined when disabled at compile time. 3) Even > if we fixed the undefined problem, |'localStorage' in window| would still > return true. > > We've been discussing these issues in a WebKit-dev thread ( > https://lists.webkit.org/pipermail/webkit-dev/2009-September/thread.html#9860) > and although (2) is probably something we can solve without too much effort > (Drew is going to look into it), (1) and (3) probably aren't worth changing > if the runtime flag is just temporary. > > *As such, I feel fairly strongly that we should start disabling features > behing a run-time flag with compile-time flags in future beta/stable builds > if they have any side-effects in JavaScript.* I'm working with Anthony > LaForge to fix this for LocalStorage/SessionStorage right now. I'm not sure > if it's worth doing preemptively for other features, but it might be. I > definitely think we should do it the next time we cut a beta build. > Especially if there's a chance the beta will be an ancestor of a stable > channel release. > > J > > > > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
