[Webkit-unassigned] [Bug 36481] Web Inspector: Edit Tag Names

[bugzilla-daemon at webkit.org]

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


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51811|review?                     |review-
               Flag|                            |




--- Comment #11 from Pavel Feldman <pfeldman at chromium.org>  2010-03-27 00:16:15 PST ---
(From update of attachment 51811)
> +void InspectorBackend::addChildElement(long callId, long nodeId)
> +{

This code should be in InspectorDOMAgent that is responsible for mutating DOM.
See other dom-related methods in backend. InspectorBackend also should not be
HTMLNames aware. Sorry if I missed it previously.
InspectorBackend is just a facade to InspectorController from the front-end
side. It only handles trivial InspectorController duties in place. Heavy-weight
stuff goes either into InspectorController or InspectorDOMAgent or to where
appropriate.

> +void InspectorBackend::changeTagName(long callId, long nodeId, const AtomicString& tagName)

Ditto. Please look at other DOM-related methods.

> +        if (WebInspector.ElementsTreeElement.EditTagBlacklist.indexOf(this.representedObject.localName) === -1) {

Nit: I got used to using maps for these even if there are not too many items.
Appending .keySet() to array definition makes a set.

> +        contextMenu.appendItem(WebInspector.UIString("Add Child Element"), this._addChildElement.bind(this));

Please do not ignore my concerns about adding text nodes or mixed content. It
really bothers me and I think I have a strong point.

> +            // FIXME: Short delay for the UI to catch up.
> +            setTimeout(function() {

See suggestions on the bug comments.

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