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
: WebKit
Web Inspector (Deprecated)
: 523.x (Safari 3)
: All All
: P2 Normal
Assigned To:
:
: InRadar
:
:
  Show dependency treegraph
 
Reported: 2007-06-26 05:34 PST by
Modified: 2009-07-24 21:39 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2007-06-26 05:34:18 PST
Dragging should be allowed from the sidebar resource list, and the URL of the resource should be in the drag data.
------- Comment #1 From 2008-01-29 11:01:52 PST -------
<rdar://problem/5712837>
------- Comment #2 From 2009-07-17 23:10:13 PST -------
Created an attachment (id=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 From 2009-07-17 23:11:45 PST -------
Created an attachment (id=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 From 2009-07-17 23:20:37 PST -------
Created an attachment (id=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 From 2009-07-23 11:33:31 PST -------
(From update of attachment 33008 [details])
> +        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 From 2009-07-23 13:42:11 PST -------
Created an attachment (id=33365) [details]
Fixed Based on Review
------- Comment #7 From 2009-07-23 13:44:18 PST -------
(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 From 2009-07-23 14:46:19 PST -------
(From update of attachment 33365 [details])
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 From 2009-07-23 15:16:37 PST -------
Created an attachment (id=33382) [details]
Added ChangeLog and Improved Loop

Added a ChangeLog and better loop.
------- Comment #10 From 2009-07-24 01:22:56 PST -------
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 From 2009-07-24 21:29:15 PST -------
Created an attachment (id=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 From 2009-07-24 21:39:13 PST -------
(From update of attachment 33485 [details])
Agreed.