[webkit-reviews] review requested: [Bug 27514] add support for watched expressions : [Attachment 39458] proposed patch - 2009/09/11

[bugzilla-daemon at webkit.org]

Patrick Mueller <pmuellr at yahoo.com> has asked  for review:
Bug 27514: add support for watched expressions
https://bugs.webkit.org/show_bug.cgi?id=27514

Attachment 39458: proposed patch - 2009/09/11
https://bugs.webkit.org/attachment.cgi?id=39458&action=review

------- Additional Comments from Patrick Mueller <pmuellr at yahoo.com>
Refactored version of previous patch, to subclass ObjectPropertiesSection and
friends rather than use it compositionally.

The code is smaller than before, but less clear, as it relies on intimate
knowledge superclass/framework behavior (my opinion).

Two changes were required in ObjectPropertiesSection.js:

- needed to allow additional customization of the tree element constructor - in
this case, I needed a special class for the root level entries in the tree (the
expressions), which have different behaviour than properties.  This could be
shaped differently somehow, haven't thought about the best way to do it -
perhaps something like a Strategy object.  The change I've made should make it
easier for people who need a unique tree element constructor in the roots
rather than below the roots.

- second change was to make nameElement an "instance variable" instead of a
local, as I need access to it later.  As a local, the reference to it was lost
after it was created.  Note that StylesSidebarPane.js  also appears to creates
an instance of nameElement in the same vein, but that class doesn't use
ObjectPropertiesSection, so there's no conflict.  I saw no other potential
conflcts for exposing this property.

There are some comments in WatchExpressionSidebarPane.js for things that didn't
feel quite right, or were working around some existing bugs.