[webkit-reviews] review denied: [Bug 27673] Inspector: Tab Through Attributes When Editing : [Attachment 33510] Tab Through CSS Properties (Needs First Patch)

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Joseph Pecoraro
<joepeck02 at gmail.com>'s request for review:
Bug 27673: Inspector: Tab Through Attributes When Editing
https://bugs.webkit.org/show_bug.cgi?id=27673

Attachment 33510: Tab Through CSS Properties (Needs First Patch)
https://bugs.webkit.org/attachment.cgi?id=33510&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> +		   if (elem.className === "name") {

This would be safer as "elem.hasStyleClass("name")" so if we add more to the
className later it still works.

> +	       // Find the attribute we were supposed to start editing in this
section
> +	       var list =
section.propertiesTreeOutline._childrenListNode.childNodes;
> +	       for (var i = 0, len = list.length; i < len; ++i) {
> +		   var listItem = list[i];
> +		   var spans = listItem.getElementsByTagName("span");
> +		   if (spans[0].textContent === nameOfPropretyToEdit) {
> +		       var event = document.createEvent("MouseEvents");
> +		       event.initMouseEvent("dblclick", true, true);
> +		       spans[1].dispatchEvent(event); // the value
> +		       return;
> +		   }
> +	       }
> +	   }

Over all these patches look good, but I think they are too low level and
fragile at this point. They are working with the DOM too much when they should
just be working with the high-level TreeElements and TreeOutlines. This would
require additions to StylePropertyTreeElement like "editName" or "editValue"
that would start editing on one of the parts without the need for a fake event
and getElementsByTagName or using private properties like _childrenListNode.