Dragging a resource in the sidebar should drag it's URL

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Timothy Hatcher]

Dragging should be allowed from the sidebar resource list, and the URL of the resource should be in the drag data.

[Adam Roben (:aroben)]

<rdar://problem/5712837>

[Joseph Pecoraro]

Created attachment 33008 [details]
Make Resource Have a Draggable URL

This wraps the TreeElement's contents in a <a href="resource url"> so that you can drag around the link as though it were a normal <a> tag.  Dragging the url into a non-inspector window works fine.

NOTE:
- This changes the Right-Click menu for the Sidebar Elements (I will attach a screenshot of the before/after behavior)
- This works fine with the double click to open patch (see patch on https://bugs.webkit.org/show_bug.cgi?id=14409)

[Joseph Pecoraro]

Created attachment 33009 [details]
Benchmarks for Different DOM Manipulation Strats

Just a quick benchmark I did that showed the current approach was faster then quickly modifying the innerHTML.  Can you think of some other ways that might be faster?  Maybe a cloneNode and changing the element type (is that possible?).

[Joseph Pecoraro]

Created attachment 33010 [details]
Before+After Right Click Menus on Resource Element

This shows the Before (left) and After (right) changes to the Right-Click Menu on a Resource Sidebar Element.  Not shown is the normal Right-Click menu of just "Reload" and "Inspect Element".  It seems though that if you choose reload this new Right-Click Menu appears with many more options.

Before: the sidebar element is a mixture of <img>, <span>, <div> which is unselectable text, but shows a textual Right-Click menu, or at least the normal menu.

After: the sidebar element is completely filled with an <a> element and thus the Right-Click menu for an <a> tag shows up.

My thoughts are this doesn't make much of a difference.  Except the fact that most of the new menu items don't work.  See https://bugs.webkit.org/show_bug.cgi?id=26881 for details on this.

[Timothy Hatcher]

Comment on attachment 33008 [details]
Make Resource Have a Draggable URL

> +        var link = document.createElement('a');
> +        link.href = this.resource.url;
> +        link.className = 'invisible';
> +        for (var i = 0, len = this._listItemNode.children.length; i < len; ++i)
> +            link.appendChild(this._listItemNode.firstChild);
> +        this._listItemNode.appendChild(link);

This should use childNodes and not children. You could do this another way using DOMRange's surroundContents() function.

https://developer.mozilla.org/en/DOM:range.surroundContents

Also use double quotes for strings.

[Joseph Pecoraro]

Created attachment 33365 [details]
Fixed Based on Review

[Joseph Pecoraro]

(In reply to comment #5)
> This should use childNodes and not children.

Done.

> Also use double quotes for strings.

Done.

You could do this another way
> using DOMRange's surroundContents() function.
> https://developer.mozilla.org/en/DOM:range.surroundContents

My tests show that range.selectNode() only works if the Node is linked into the actual DOM.  In this case the list item has not yet been added to the DOM.  If you think we should move surrounding it as a link somewhere else let me know, but for now the surroundContents approach won't work.

[Timothy Hatcher]

Comment on attachment 33365 [details]
Fixed Based on Review

Sorry to keep nitpicking, but this for loop could be a while loop like:

while (this._listItemNode.hasChildNodes())
   link.appendChild(this._listItemNode.firstChild);

[Joseph Pecoraro]

Created attachment 33382 [details]
Added ChangeLog and Improved Loop

Added a ChangeLog and better loop.

[Adam Barth]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/ResourcesPanel.js
	M	WebCore/inspector/front-end/inspector.css
Committed r46337
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/ResourcesPanel.js
	M	WebCore/inspector/front-end/inspector.css
r46337 = 2fd89022eb678166cb159bfe02892bce4445959d (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46337

[Joseph Pecoraro]

Created attachment 33485 [details]
More Consistent Style

Changed the element to have "cursor:default" which was the behavior it had before, but the <a> turned it into a pointer.  I think it looks nicer and more consistent to have a the default cursor.  What do you think?

[Timothy Hatcher]

Comment on attachment 33485 [details]
More Consistent Style

Agreed.