Bug 143814 - Web Inspector: Allow toggling the edibility of a DOMTreeOutline
Summary: Web Inspector: Allow toggling the edibility of a DOMTreeOutline
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-04-15 21:18 PDT by Joseph Pecoraro
Modified: 2015-04-16 13:34 PDT (History)
9 users (show)

See Also:

Attachments
[PATCH] Proposed Fix (8.52 KB, patch)
2015-04-15 21:27 PDT, Joseph Pecoraro 		burg: review-
Details | Formatted Diff | Diff
[PATCH] Proposed Fix (9.02 KB, patch)
2015-04-16 13:04 PDT, Joseph Pecoraro 		burg: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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