[webkit-reviews] review denied: [Bug 43434] Web Inspector: Relative URL Link Tooltips do not respect <base> : [Attachment 164908] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 43434: Web Inspector: Relative URL Link Tooltips do not respect <base>
https://bugs.webkit.org/show_bug.cgi?id=43434

Attachment 164908: Patch
https://bugs.webkit.org/attachment.cgi?id=164908&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=164908&action=review


> Source/WebCore/inspector/Inspector.json:1569
>		       { "name": "documentURL", "type": "string", "optional":
true, "description": "Document URL that <code>Document</code> or
<code>FrameOwner</code> node points to." },

Could you look up what we use the documentURL for today?

> Source/WebCore/inspector/InspectorDOMAgent.cpp:1182
> +    baseURL = ((baseURL.isEmpty() || baseURL == blankURL()) &&
document->parentDocument()) ? document->parentDocument()->baseURL() : baseURL;

Could you use document::completeURL("") to make sure we are consistent with
core?

> Source/WebCore/inspector/InspectorDOMAgent.cpp:1250
> +	   String baseURL = documentBaseURLString(document);

I'd suggest to always send baseURL for documents.