Bug 31142 - DOM tree selection disappears upon page refresh
Summary: DOM tree selection disappears upon page refresh
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
Keywords: InRadar
Depends on:
Reported: 2009-11-04 14:25 PST by Piotr Petrus
Modified: 2009-12-02 10:32 PST (History)
10 users (show)

See Also:

[PATCH] Proposed fix (7.77 KB, patch)
2009-11-30 13:58 PST, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff
[PATCH] Now with no 'animation'. (18.39 KB, patch)
2009-12-02 07:32 PST, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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,
Comment 1 Timothy Hatcher 2009-11-04 14:28:38 PST
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