Web Inspector: InspectorBackend.loadFromJSONIfNeeded should take the JSON url as argument

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

[Vivek Galatage]

The method loadFromJSONIfNeeded of InspectorBackend in InspectorBackend.js should take the JSON url as input.

This is needed for the Inspector Protocol test harness to load as the test will be residing in a different location.

Patch to follow.

[Vivek Galatage]

Created attachment 163523 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 163523 [details]
Patch

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

> Source/WebCore/inspector/front-end/inspector.js:409
> +    var jsonUrl = location.href.slice(0, location.href.lastIndexOf("/")) + "/../Inspector.json";

It will fail on URL like this: http://localhost/inspector/front-end/inspector.html?ws=localhost:9222/devtools/page/2_1

Please test this with the instructions described here: https://groups.google.com/d/msg/google-chrome-developer-tools/L1nemsmtnJI/ndP4cYkTy-sJ

[Yury Semikhatsky]

Bt(In reply to comment #2)
> (From update of attachment 163523 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163523&action=review
> 
> > Source/WebCore/inspector/front-end/inspector.js:409
> > +    var jsonUrl = location.href.slice(0, location.href.lastIndexOf("/")) + "/../Inspector.json";
> 
> It will fail on URL like this: http://localhost/inspector/front-end/inspector.html?ws=localhost:9222/devtools/page/2_1
> 
> Please test this with the instructions described here: https://groups.google.com/d/msg/google-chrome-developer-tools/L1nemsmtnJI/ndP4cYkTy-sJ

btw, what was wrong with '../Inspector.json' ?

[Vivek Galatage]

(In reply to comment #3)
> btw, what was wrong with '../Inspector.json' ?

The call to above location is being made from inspector.js which is included in inspector.html in Source/WebCore/inspector/front-end. So this will work if its getting called from the inspector.html.

But in case of the testing harness, the files protocol-test.html and protocol-test.js will be placed somewhere e.g. LayoutTests/http/tests/inspector-protocol folder. So when the call to loadFromJSONIfNeeded is being made from these places, the system will not find the path to the ../Inspector.json. Hence the call site should give the json location relative to where its placed.

[Vivek Galatage]

(In reply to comment #2)
> (From update of attachment 163523 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163523&action=review
> 
> > Source/WebCore/inspector/front-end/inspector.js:409
> > +    var jsonUrl = location.href.slice(0, location.href.lastIndexOf("/")) + "/../Inspector.json";
> 
> It will fail on URL like this: http://localhost/inspector/front-end/inspector.html?ws=localhost:9222/devtools/page/2_1
> 
> Please test this with the instructions described here: https://groups.google.com/d/msg/google-chrome-developer-tools/L1nemsmtnJI/ndP4cYkTy-sJ

Or even simple we can just pass "../Inspector.json" as url is already done i.e.

var jsonUrl = "../Inspector.json";
InspectorBackend.loadFromJSONIfNeeded(jsonUrl);

Without any change with current implementation.

[Yury Semikhatsky]

(In reply to comment #4)
> (In reply to comment #3)
> > btw, what was wrong with '../Inspector.json' ?
> 
> The call to above location is being made from inspector.js which is included in inspector.html in Source/WebCore/inspector/front-end. So this will work if its getting called from the inspector.html.
> 
> But in case of the testing harness, the files protocol-test.html and protocol-test.js will be placed somewhere e.g. LayoutTests/http/tests/inspector-protocol folder. So when the call to loadFromJSONIfNeeded is being made from these places, the system will not find the path to the ../Inspector.json. Hence the call site should give the json location relative to where its placed.

Exactly. But it will only affect the new code. Current implementation can keep using relative URL.

(In reply to comment #5)
> (In reply to comment #2)
> > (From update of attachment 163523 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=163523&action=review
> > 
> > > Source/WebCore/inspector/front-end/inspector.js:409
> > > +    var jsonUrl = location.href.slice(0, location.href.lastIndexOf("/")) + "/../Inspector.json";
> > 
> > It will fail on URL like this: http://localhost/inspector/front-end/inspector.html?ws=localhost:9222/devtools/page/2_1
> > 
> > Please test this with the instructions described here: https://groups.google.com/d/msg/google-chrome-developer-tools/L1nemsmtnJI/ndP4cYkTy-sJ
> 
> Or even simple we can just pass "../Inspector.json" as url is already done i.e.
> 
> var jsonUrl = "../Inspector.json";
> InspectorBackend.loadFromJSONIfNeeded(jsonUrl);
> 
> Without any change with current implementation.
Exactly.

[Vivek Galatage]

Created attachment 163551 [details]
Patch

[WebKit Review Bot]

Comment on attachment 163551 [details]
Patch

Clearing flags on attachment: 163551

Committed r128287: <http://trac.webkit.org/changeset/128287>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.