Bug 43434

Summary:

Web Inspector: Relative URL Link Tooltips do not respect <base>

Product:

WebKit

Reporter:

Joseph Pecoraro <joepeck>

Component:

Web Inspector (Deprecated)

Assignee:

Alexander Pavlov (apavlov) <apavlov>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, bweinstein, joepeck, keishi, pfeldman, pmuellr, rik, yurys

Priority:

Keywords:

HasReduction

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
[TEST] Basic base-test.html	none
[TEST] base-test-iframe.html	none
Patch	none
Patch	none
Patch	pfeldman: review+

Joseph Pecoraro

Reported 2010-08-03 10:35:49 PDT

Created attachment 63348 [details] [TEST] Basic base-test.html Tooltips for relative URL links show up incorrectly. They appear to show using the documentURL instead of the <base> if one is defined. The HTML5 spec also mentions that xml:base could affect the real link in the end. Example: (file:///Users/joe/Desktop/base-test.html) <!DOCTYPE html> <html> <head> <title>This is an example for the <base> element</title> <base href="http://www.example.com/news/index.html"> </head> <body> <p>Visit the <a href="archives.html">archives</a>.</p> </body> </html> Hovering the link in the Elements heirarchy gives: [incorrect] file:///Users/joe/Desktop/archives.html Clicking the link gives: [correct] http://www.example.com/news/archives.html Attached test case for a basic page, and another using an iframe.

Attachments
[TEST] Basic base-test.html (244 bytes, text/html) 2010-08-03 10:35 PDT, Joseph Pecoraro	no flags	Details
[TEST] base-test-iframe.html (221 bytes, text/html) 2010-08-03 10:37 PDT, Joseph Pecoraro	no flags	Details
Patch (11.51 KB, patch) 2012-09-20 06:39 PDT, Alexander Pavlov (apavlov)	no flags	Details Formatted Diff Diff
Patch (11.26 KB, patch) 2012-09-24 05:44 PDT, Alexander Pavlov (apavlov)	no flags	Details Formatted Diff Diff
Patch (11.22 KB, patch) 2012-09-24 06:15 PDT, Alexander Pavlov (apavlov)	pfeldman: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Joseph Pecoraro

Comment 1 2010-08-03 10:37:23 PDT

Created attachment 63349 [details] [TEST] base-test-iframe.html This tests with an <iframe> containing the basic base-test, as it might have its own <base>.

Alexander Pavlov (apavlov)

Comment 2 2011-11-23 07:31:19 PST

(In reply to comment #1) > Created an attachment (id=63349) [details] > [TEST] base-test-iframe.html > > This tests with an <iframe> containing the basic base-test, as > it might have its own <base>. I've got a patch for this, but it appears that a document's base URL can be updated at any time, and with iframes it happens AFTER the iframed document has been pushed into the frontend (which is the moment its baseURL was retrieved for pushing into the frontend), so we tentatively need to introduce a corresponding notification event.

Alexander Pavlov (apavlov)

Comment 3 2012-09-20 06:39:33 PDT

Created attachment 164908 [details] Patch

Pavel Feldman

Comment 4 2012-09-21 04:33:07 PDT

Comment on attachment 164908 [details] Patch 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.

Alexander Pavlov (apavlov)

Comment 5 2012-09-24 05:43:44 PDT

(In reply to comment #4) > (From update of attachment 164908 [details]) > 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? Lots of necessary things... > > 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? Done. > > Source/WebCore/inspector/InspectorDOMAgent.cpp:1250 > > + String baseURL = documentBaseURLString(document); > > I'd suggest to always send baseURL for documents. Done.

Alexander Pavlov (apavlov)

Comment 6 2012-09-24 05:44:03 PDT

Created attachment 165360 [details] Patch

Alexander Pavlov (apavlov)

Comment 7 2012-09-24 06:15:49 PDT

Created attachment 165365 [details] Patch

Pavel Feldman

Comment 8 2012-09-24 09:35:41 PDT

Comment on attachment 165365 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165365&action=review > Source/WebCore/inspector/InspectorDOMAgent.cpp:1247 > + String documentURL = documentURLString(document); Nit: no need for these intermediate variables.

Alexander Pavlov (apavlov)

Comment 9 2012-09-25 02:01:41 PDT

Committed r129477: <http://trac.webkit.org/changeset/129477>

Note You need to log in before you can comment on or make changes to this bug.