Bug 31142

Summary: DOM tree selection disappears upon page refresh
Product: WebKit Reporter: Piotr Petrus <itunesgripes>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, bweinstein, joepeck, keishi, pfeldman, pmuellr, riddlu, rik, timothy, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed fix
timothy: review+
[PATCH] Now with no 'animation'. timothy: review+

Description Piotr Petrus 2009-11-04 14:25:04 PST 
Web Inspector doesn’t restore selection for DOM element (HTML tree in the Elements pane) upon page refresh. This is crucial for front-end developers, because we develop sites locally and rely on instant feedback in the browser for any minor change done in the source code (may it be CSS, HTML or JavaScript).

Steps to reproduce: 

1. Open the latest WebKit.app, and right-click on "How can I help" header from the default homepage and choose "Inspect Element"
2. Web Inspector will open and <h2> element will be selected
3. Press Cmd+R to refresh that page
4. Web Inspector will not close, but the selection will be gone – going back to <body> element

What is supposed to happen: If the selected element is still present in the DOM after page refresh, select it again after refresh.

For any other feedback / ideas, I suggest running Firefox + Firebug. They’ve implemented it.

Thanks a lot,
Peter
Comment 1 Timothy Hatcher 2009-11-04 14:28:38 PST 
<rdar://problem/4987510>
Comment 2 Pavel Feldman 2009-11-30 13:58:04 PST 
Created attachment 44041 [details]
[PATCH] Proposed fix
Comment 3 Adam Barth 2009-11-30 14:03:06 PST 
style-queue ran check-webkit-style on attachment 44041 [details] without any errors.
Comment 4 Pavel Feldman 2009-12-02 07:32:36 PST 
Created attachment 44145 [details]
[PATCH] Now with no 'animation'.

Couple of drive-by improvements here.
Comment 5 WebKit Review Bot 2009-12-02 07:33:35 PST 
style-queue ran check-webkit-style on attachment 44145 [details] without any errors.
Comment 6 Timothy Hatcher 2009-12-02 09:34:32 PST 
Comment on attachment 44145 [details]
[PATCH] Now with no 'animation'.

> +    for (size_t i = 0; i < pathTokens.size()  - 1; i += 2) {

Extra space before "- 1".


> +        int childNumber = pathTokens[i].toInt(&success);
> +        int nodesCount = innerChildNodeCount(node);

Can these be unsigned/size_t?


> +        String childName = pathTokens[i + 1];

Move this down to where you need it.


> +        for (int j = 0; child && j < childNumber; ++j)

j should be size_t.


>          ~InspectorDOMAgent();
> +        void reset();

Put a newline before reset so it isn't grouped with the destructor.


> +        this.reset();
> +        if (!inspectedRootDocument)
>              return;

Put a newline after the reset call.


> +            InjectedScriptAccess.pushNodeByPathToFrontend(this._selectedPathOnReset, selectLastSelectedNode.bind(this));

It seems odd to call a function called "pushNodeByPathToFrontend" from the frontend! Should it just be "nodeByPath" and take a callback?
Comment 7 Pavel Feldman 2009-12-02 10:32:19 PST 
Landed with comments addressed.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/inspector/elements-panel-selection-on-refresh-expected.txt
	A	LayoutTests/inspector/elements-panel-selection-on-refresh.html
	M	WebCore/ChangeLog
	M	WebCore/inspector/InjectedScriptHost.cpp
	M	WebCore/inspector/InjectedScriptHost.h
	M	WebCore/inspector/InjectedScriptHost.idl
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorDOMAgent.cpp
	M	WebCore/inspector/InspectorDOMAgent.h
	M	WebCore/inspector/front-end/DOMAgent.js
	M	WebCore/inspector/front-end/ElementsPanel.js
	M	WebCore/inspector/front-end/InjectedScript.js
	M	WebCore/inspector/front-end/InjectedScriptAccess.js
Committed r51601