Bug 14410 - Dragging a resource in the sidebar should drag it's URL
: Dragging a resource in the sidebar should drag it's URL
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated)
: 523.x (Safari 3)
: All All
: P2 Normal
Assigned To: Nobody
: InRadar
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-26 05:34 PDT by Timothy Hatcher
Modified: 2009-07-24 21:39 PDT (History)
4 users (show)

See Also:


Attachments
Make Resource Have a Draggable URL (2.10 KB, patch)
2009-07-17 23:10 PDT, Joseph Pecoraro
timothy: review-
Details | Formatted Diff | Diff
Benchmarks for Different DOM Manipulation Strats (1.51 KB, application/x-javascript)
2009-07-17 23:11 PDT, Joseph Pecoraro
no flags Details
Before+After Right Click Menus on Resource Element (226.28 KB, image/png)
2009-07-17 23:20 PDT, Joseph Pecoraro
no flags Details
Fixed Based on Review (1.30 KB, patch)
2009-07-23 13:42 PDT, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff
Added ChangeLog and Improved Loop (1.82 KB, patch)
2009-07-23 15:16 PDT, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff
More Consistent Style (855 bytes, patch)
2009-07-24 21:29 PDT, Joseph Pecoraro
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2007-06-26 05:34:18 PDT
Dragging should be allowed from the sidebar resource list, and the URL of the resource should be in the drag data.
Comment 1 Adam Roben (:aroben) 2008-01-29 11:01:52 PST
<rdar://problem/5712837>
Comment 2 Joseph Pecoraro 2009-07-17 23:10:13 PDT
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)
Comment 3 Joseph Pecoraro 2009-07-17 23:11:45 PDT
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?).
Comment 4 Joseph Pecoraro 2009-07-17 23:20:37 PDT
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.
Comment 5 Timothy Hatcher 2009-07-23 11:33:31 PDT
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.
Comment 6 Joseph Pecoraro 2009-07-23 13:42:11 PDT
Created attachment 33365 [details]
Fixed Based on Review
Comment 7 Joseph Pecoraro 2009-07-23 13:44:18 PDT
(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.
Comment 8 Timothy Hatcher 2009-07-23 14:46:19 PDT
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);
Comment 9 Joseph Pecoraro 2009-07-23 15:16:37 PDT
Created attachment 33382 [details]
Added ChangeLog and Improved Loop

Added a ChangeLog and better loop.
Comment 10 Adam Barth 2009-07-24 01:22:56 PDT
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
Comment 11 Joseph Pecoraro 2009-07-24 21:29:15 PDT
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?
Comment 12 Timothy Hatcher 2009-07-24 21:39:13 PDT
Comment on attachment 33485 [details]
More Consistent Style

Agreed.