Bug 143814

Summary:

Web Inspector: Allow toggling the edibility of a DOMTreeOutline

Product:

WebKit

Reporter:

Joseph Pecoraro <joepeck>

Component:

Web Inspector

Assignee:

Joseph Pecoraro <joepeck>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

burg, commit-queue, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
[PATCH] Proposed Fix	burg: review-
[PATCH] Proposed Fix	burg: review+

Description Joseph Pecoraro 2015-04-15 21:18:09 PDT

* SUMMARY
Allow toggling the edibility of a DOMTreeOutline.

A DOMTreeOutline in the console should not be editable. You can "Reveal in DOM Tree" to edit from the DOM tree. ObjectTree's themselves are not editable right now.
A DOMTreeContentView should always have editable DOMTreeOutlines.

Comment 1 Radar WebKit Bug Importer 2015-04-15 21:18:22 PDT

<rdar://problem/20565653>

Comment 2 Joseph Pecoraro 2015-04-15 21:27:08 PDT

Created attachment 250896 [details]
[PATCH] Proposed Fix

Comment 3 Joseph Pecoraro 2015-04-15 21:33:32 PDT

Patch doesn't apply because it relies on an earlier change in very similar code.

Comment 4 Brian Burg 2015-04-16 08:20:42 PDT

Comment on attachment 250896 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=250896&action=review

Very interesting, I made nearly the exact same change in my branch. (Snapshotted DOM nodes aren't editable either.)

It seems you missed DOMTreeElement._canAddAttributes (set in constructor) and disabling draggability (DOM elements are configured in onattach). Once these cases are fixed this patch should be ready to go.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js:146
> +    set editable(x)

I would prefer 'value' rather than 'x'.

Comment 5 Joseph Pecoraro 2015-04-16 11:56:34 PDT

(In reply to comment #4)
> Comment on attachment 250896 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250896&action=review
> 
> Very interesting, I made nearly the exact same change in my branch.
> (Snapshotted DOM nodes aren't editable either.)
> 
> It seems you missed DOMTreeElement._canAddAttributes (set in constructor)
> and disabling draggability (DOM elements are configured in onattach). Once
> these cases are fixed this patch should be ready to go.

Great catches!

Comment 6 Joseph Pecoraro 2015-04-16 13:02:50 PDT

> It seems you missed DOMTreeElement._canAddAttributes (set in constructor)

This seems like it should stay unaffected. This essentially distinguishes that this DOMTreeElement is the opening tag of an element, so if it was editable attributes can be added. Perhaps it should be renamed.

> and disabling draggability (DOM elements are configured in onattach).

Yep!

Comment 7 Joseph Pecoraro 2015-04-16 13:04:16 PDT

Created attachment 250943 [details]
[PATCH] Proposed Fix

Comment 8 Joseph Pecoraro 2015-04-16 13:09:41 PDT

> > Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js:146
> > +    set editable(x)
> 
> I would prefer 'value' rather than 'x'.

Oops, I forgot about this. We tend to use x pretty consistently though. Especially for short functions.

Comment 9 Brian Burg 2015-04-16 13:26:11 PDT

Comment on attachment 250943 [details]
[PATCH] Proposed Fix

r=me

Comment 10 Joseph Pecoraro 2015-04-16 13:34:50 PDT

http://trac.webkit.org/changeset/182905