Bug 89554 – Web Inspector: Having a "Scroll into view" for nodes through web inspector.

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

[sam]

Web inspector currently does not have an option for getting the node to scroll into view.

[sam]

I am looking into this and will upload the patch soon.
Also attached is the screen shot for illustration purpose.
Plz do let me know if there are any suggestions/comments.

[sam]

Created attachment 148525 [details]
screenshot illustrating scroll into view option in menu.

[Yury Semikhatsky]

This can be achieved by evaluating "$0.scrollIntoViewIfNeeded(true)" in the console. I'm not sure if it's worth a context menu action.

[Pavel Feldman]

I like the context menu part, it does not clutter UI, user does not need to be aware of console API. I would put it lower though, so that it was below all edit / copy operations.

[sam]

Thanks Yury, Pavel for your feedbacks.

I was just wondering if including a new api to execute expressions through console will be ok. This would basically a very tiny one - work similar to executeThroughConsoleUsingTextPrompt but will neither set record  nor print result and will also not show console panel - only direct execution.
I guess it might be used in later use cases also.

Or is there something else also which I might be missing/getting wrong?

[Yury Semikhatsky]

(In reply to comment #5)
> Thanks Yury, Pavel for your feedbacks.
> 
> I was just wondering if including a new api to execute expressions through console will be ok. This would basically a very tiny one - work similar to executeThroughConsoleUsingTextPrompt but will neither set record  nor print result and will also not show console panel - only direct execution.
> I guess it might be used in later use cases also.
> 
> Or is there something else also which I might be missing/getting wrong?

What do you mean by "including a new api"? We already have RuntimeAgent.evaluate(...) method on the front-end that allows you to perform evaluation in the inspected page, current console UI invokes that method, isn't it enough for your needs?

[sam]

Created attachment 149009 [details]
Patch

[sam]

(In reply to comment #6)
> (In reply to comment #5)
> > Thanks Yury, Pavel for your feedbacks.
> > 
> > I was just wondering if including a new api to execute expressions through console will be ok. This would basically a very tiny one - work similar to executeThroughConsoleUsingTextPrompt but will neither set record  nor print result and will also not show console panel - only direct execution.
> > I guess it might be used in later use cases also.
> > 
> > Or is there something else also which I might be missing/getting wrong?
> 
> What do you mean by "including a new api"? We already have RuntimeAgent.evaluate(...) method on the front-end that allows you to perform evaluation in the inspected page, current console UI invokes that method, isn't it enough for your needs?

Ya true, i just meant to include a wrapper like executeThroughConsoleUsingTextPrompt, but later realized that eval can be done by direct call.
Thank you Yury.

[Yury Semikhatsky]

Comment on attachment 149009 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149009&action=review

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:1073
> +            var expression = "$0.scrollIntoView(true)";

Note that $0 may not necessarily be a DOM node, e.g. it may be last element selected in a heap profile.

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:1074
> +            WebInspector.consoleView.evalInInspectedWindow(expression, "console", true, false, false, null);

Instead of going through the consoleView, you should be able to get current selected node and evaluate .scrollIntoView(true) on it using RuntimeAgent.evaluate

[Pavel Feldman]

Comment on attachment 149009 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=149009&action=review

>> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:1074
>> +            WebInspector.consoleView.evalInInspectedWindow(expression, "console", true, false, false, null);
> 
> Instead of going through the consoleView, you should be able to get current selected node and evaluate .scrollIntoView(true) on it using RuntimeAgent.evaluate

Here is the hint on how to do what Yury suggests:

WebInspector.RemoteObject.resolveNode(nodeId, "", callback);

function callback(object)
{
    function scrollIntoView()
    {
        this.scrollIntoView(true);
    }

    if (object)
        object.callFunction(scrollIntoView, []);
}

[sam]

Created attachment 150877 [details]
Patch

[Pavel Feldman]

Comment on attachment 150877 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150877&action=review

Looks good. Couple of nits and it is ready to land.

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:1920
> +    scrollIntoView: function()

Should be private (_scrollIntoView)

> Source/WebCore/inspector/front-end/ElementsTreeOutline.js:1930
> +                object.callFunction(scrollIntoView, undefined);

No need to pass undefined, just declare this parameter as optional.

> Source/WebCore/inspector/front-end/RemoteObject.js:287
> +            if (!callback)

You need to declare callback optional in the jsdoc.

[sam]

Created attachment 150899 [details]
Patch

[Pavel Feldman]

Comment on attachment 150899 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150899&action=review

> Source/WebCore/inspector/front-end/RemoteObject.js:275
> +     * @param {(Array.<RuntimeAgent.CallArgument>|undefined)=} args

no need for "|undefined" now that this parameter is optional.

[sam]

Created attachment 150903 [details]
Patch

[sam]

(In reply to comment #14)
> (From update of attachment 150899 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150899&action=review
> 
> > Source/WebCore/inspector/front-end/RemoteObject.js:275
> > +     * @param {(Array.<RuntimeAgent.CallArgument>|undefined)=} args
> 
> no need for "|undefined" now that this parameter is optional.

err, will upload it again. Thanks for the review Pavel.

[Pavel Feldman]

Comment on attachment 150903 [details]
Patch

Thanks!

[WebKit Review Bot]

Comment on attachment 150903 [details]
Patch

Clearing flags on attachment: 150903

Committed r121896: <http://trac.webkit.org/changeset/121896>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.