On Fri, Feb 24, 2012 at 12:07 PM, Maciej Stachowiak <[email protected]> wrote: > > On Feb 24, 2012, at 11:58 AM, Adam Barth wrote: > >> On Fri, Feb 24, 2012 at 11:50 AM, Maciej Stachowiak <[email protected]> wrote: >>> On Feb 24, 2012, at 10:27 AM, Adam Barth wrote: >>>> 2012/2/24 Maciej Stachowiak <[email protected]>: >>>>> I too am surprised that HTML-related APIs would be refactored as a result >>>>> of modularization. This change may be justifiable on its own merits, but >>>>> it doesn't seem like a logical part of a project to make self-contained >>>>> features more modular. At the very least, to avoid confusion, changes >>>>> like that should be kept clearly separate from the modularization effort, >>>>> or else, someone could explain the relationship if there is one and its >>>>> not obvious. >>>> >>>> Fair enough. I've detached those bugs from the larger meta bug. >>>> >>>> These patches have a different goal than the other patches attached to >>>> that meta bug. Much in the same way that we've moved code out of >>>> Frame.h over time, these patches are intended to make DOMWindow.idl >>>> more readable. The net result will (hopefully!) be a file that's more >>>> focused on concerns that actually relate to DOMWindow (e.g., >>>> name/closed/opener/parent/top) rather than a dumping ground for every >>>> random thing that needs to be in the global scope. >>>> >>>> In retrospect, we should have presented this work separately so folks >>>> could have discussed its merits separately. I think we got tied up in >>>> the implementation detail that the same mechanism makes both projects >>>> possible. >>> >>> I think this case is a little different than Frame.h, because with Frame, >>> we can actually move related methods to separate objects, thus actually >>> splitting up the interface. This creates an actual separation of concerns >>> in the code if done right, not just a file split. I'm not sure that having >>> multiple files which actually all form a single interface is equally >>> beneficial. It doesn't seem hugely worse, but it does not seem obviously >>> better to me either. >> >> It's quite analogous to Frame.h in the sense that this mechanism also >> lets us move related methods to a separate object. >> >> Consider a case such as >> <http://trac.webkit.org/browser/trunk/Source/WebCore/fileapi/DOMWindowFileSystem.idl>. >> When we moved webkitRequestFileSystem from DOMWindow.idl to >> DOMWindowFileSystem.idl, the code for webkitRequestFileSystem moved >> from DOMWindow.cpp to >> <http://trac.webkit.org/browser/trunk/Source/WebCore/fileapi/DOMWindowFileSystem.cpp#L53>, >> separating these concerns from DOMWindow. > > Splitting out the file-related stuff from DOMWindow seems justified based on > modularity grounds, presuming we see File API as a self-contained feature. So > no objection there. > >> >> In the case of >> <http://trac.webkit.org/browser/trunk/Source/WebCore/html/DOMWindowHTML.idl>, >> the C++ code that backs those interfaces is already in >> Source/WebCore/html. The net result is 100 less lines of "noise" in >> DOMWindow.idl. > > It seems to me like the practical difference of the IDL move in this case is > changing the IDL file where you need to add HTML elements, and adding an > extra place you need to look to see what's in the global scope. It's not > obvious to me that this is an improvement, but like I said, it doesn't seem > terrible. > > I think there is a potentially even better approach here though. > HTMLTagNames.in is already used to autogenerate most of the things that need > to mention every HTML element. If it was used to avoid the need to explicitly > mention all HTML element constructors in any IDL file at all, for instance by > autogenerating one, then it would reduce the number of places that have to > mention every HTML element by 1, and eliminate one of the possible mistakes > when adding a new HTML element.
Yeah, autogenerating this code from HTMLTagNames.in would be a better solution. I look forward to your patch. Adam _______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
