[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 #51398|review?                     |review-
               Flag|                            |




--- Comment #3 from Pavel Feldman <pfeldman at chromium.org>  2010-03-23 01:06:31 PST ---
(From update of attachment 51398)
Looks cool! Menu gets populated!!!

General notes go first:
- Given a node with N children, Add Child only solves 1/N part of the problem.
I.e. Besides "Add Child" we need "Insert Above" and "Insert Below".
- I did not track it in the code, but according to the video, adding text nodes
capability is missing
- Taking #2 to the greater level, I think Add / Insert Child should be
implemented as Edit as HTML. I.e. accept free flow text as an input. That way
user would be able to paste portions of HTML as a source for new nodes. I know
we lose nice structuring of added nodes, but we gain huge flexibility in
return.



> +using namespace HTMLNames;
> +

I think our relationship with HTMLNames is not strong enough to start using
using.

> +        frontend->didRemoveNode(callId, -1);

Bug. -> didAddChild ?

> +
> +        newElem->copyNonAttributeProperties(oldElem);

Does this take care of JS properties on wrappers?

> +    if (code) {
> +        frontend->didChangeTagName(callId, -3);

Do you handle these error codes? If not, why differentiating?

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

Oh no.

I'll review front-end code in greater detail later.

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