Bug 33025 – Web Inspector: Provide contentDocument URL for frame owner elements

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

[Alexander Pavlov (apavlov)]

Currently, there is no way to determine the URL of the document a node belongs in (be it the top-level document or a nested frame document). The ownerDocument for all nodes is the top-level document and that is the only source of information.

This issue blocks https://bugs.webkit.org/show_bug.cgi?id=33024.

[Alexander Pavlov (apavlov)]

Created attachment 45616 [details]
patch

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45616 [details] without any errors.

[Timothy Hatcher]

Comment on attachment 45616 [details]
patch

Should be _url and url, per our style guide.

[Pavel Feldman]

Comment on attachment 45616 [details]
patch

> +                value.set("URL", frameOwner->contentDocument()->url().string());

As mentioned, should be "url".

> +        if (payload.URL)
> +            this._URL = payload.URL;

I think you are going to access it from outside the agent class, so should be public ("url", not "_url").

[Alexander Pavlov (apavlov)]

Created attachment 45621 [details]
patch (property name fixed)

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45621 [details] without any errors.

[Pavel Feldman]

Comment on attachment 45621 [details]
patch (property name fixed)

> +        if (payload.documentURL)
> +            this.documentURL = payload.documentURL;

Note that this has a side effect that converts "" to undefined.

[Alexander Pavlov (apavlov)]

(In reply to comment #7)
> (From update of attachment 45621 [details])
> > +        if (payload.documentURL)
> > +            this.documentURL = payload.documentURL;
> 
> Note that this has a side effect that converts "" to undefined.

That's fine, since URLs are typically not empty strings, and in our case "" makes as much sense as <undefined>...

[WebKit Commit Bot]

Comment on attachment 45621 [details]
patch (property name fixed)

Rejecting patch 45621 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueueSVN/LayoutTests
Testing 11853 test cases.
inspector/timeline-script-tag-1.html -> crashed

Exiting early after 1 failures. 9509 tests run.
398.62s total testing time

9508 test cases (99%) succeeded
1 test case (<1%) crashed
6 test cases (<1%) had stderr output

Full output: http://webkit-commit-queue.appspot.com/results/150884

[Alexander Pavlov (apavlov)]

Created attachment 45664 [details]
patch (null document handled properly)

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45664 [details] without any errors.

[Eric Seidel (no email)]

Would be best if pfeldman would like to review again.

[WebKit Commit Bot]

Comment on attachment 45664 [details]
patch (null document handled properly)

Clearing flags on attachment: 45664

Committed r52836: <http://trac.webkit.org/changeset/52836>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.