Bug 40420 – Web Inspector: Prevent from copying "filename.css" in Styles pane

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

[Nikita Vasilyev]

Screencast: http://screenr.com/YMK

[Pavel Feldman]

We should render location using pseudo elements (::after or ::before). Pseudo elements do not get into the clipboard.

[Nikita Vasilyev]

(In reply to comment #1)
How can we change a content of pseudo elements via JavaScript?

[Pavel Feldman]

(In reply to comment #2)
> (In reply to comment #1)
> How can we change a content of pseudo elements via JavaScript?

.foo::after {
	content: attr(attrBar);
}


<div class="foo" attrBar="Text"></div>

[Nikita Vasilyev]

Created attachment 58382 [details]
Uncopyable subtitle

[Pavel Feldman]

Comment on attachment 58382 [details]
Uncopyable subtitle

Couple of nits.

WebCore/inspector/front-end/Section.js:86
 +          if (x instanceof Node) {
I don't think we should extend section's api to accept both text and Nodes as subtitles. subtitle setter is a convenience method for setting text. Should client need greater extensibility, he is free to access subtitleElement explicitly.

WebCore/inspector/front-end/StylesSidebarPane.js:654
 +                  var link = WebInspector.linkifyResourceAsNode(url, "resources", this.rule.sourceLine + 1);
So I would leave this code as is with appending child here as it used to be.

WebCore/inspector/front-end/inspector.css:4041
 +  .styles-section .header [data-uncopyable]::before {
You should just apply this to ".styles-section .subtitle".

[Nikita Vasilyev]

(In reply to comment #5)
> WebCore/inspector/front-end/Section.js:86
>  +          if (x instanceof Node) {
> I don't think we should extend section's api to accept both text and Nodes as subtitles. subtitle setter is a convenience method for setting text. Should client need greater extensibility, he is free to access subtitleElement explicitly.

I borrowed it from the title setter; tried to be consistent.

> WebCore/inspector/front-end/inspector.css:4041
>  +  .styles-section .header [data-uncopyable]::before {
> You should just apply this to ".styles-section .subtitle".

And ".styles-section .subtitle a" in case of a link to CSS file.

[Nikita Vasilyev]

Created attachment 58482 [details]
Uncopyable subtitle

[Nikita Vasilyev]

> Pavel Feldman has canceled Nikita Vasilyev's request for review:
> Bug 40420: Web Inspector: Prevent from copying "filename.css" in Styles pane
> https://bugs.webkit.org/show_bug.cgi?id=40420
> 
> Attachment 58482 [details]: Uncopyable subtitle
> https://bugs.webkit.org/attachment.cgi?id=58482&action=review

What's the reason?

[Pavel Feldman]

Comment on attachment 58482 [details]
Uncopyable subtitle

Sorry, something went wrong with posting review comments.

WebCore/inspector/front-end/inspector.css:4041
 +  .styles-section .header .subtitle::before, .styles-section .header .subtitle a::before {
No need for .header here. Also, could we try to get rid of the pseudo element for <a>? Is it here for the link handler + formatting to work?

Rest looks good!

[Nikita Vasilyev]

Created attachment 58690 [details]
Uncopyable subtitle (.header removed)

> Also, could we try to get rid of the pseudo element for <a>? Is it here for the link handler + formatting to work?

No, we can't. ".styles-section .subtitle a" simply doesn't work without "::before". To be exact, it doesn't show links at all.

[WebKit Commit Bot]

Comment on attachment 58690 [details]
Uncopyable subtitle (.header removed)

Rejecting patch 58690 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--ignore-tests', 'compositing', '--quiet']" exit_code: 1
Last 500 characters of output:
e
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Skipped list contained 'compositing/iframes/composited-iframe.html', but no file of that name could be found
Testing 19115 test cases.
inspector/elements-panel-styles.html -> failed

Exiting early after 1 failures. 15406 tests run.
278.02s total testing time

15405 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
14 test cases (<1%) had stderr output

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

[Nikita Vasilyev]

(In reply to comment #11)

I wish I could fix this test, but I can't even run it. My full output from run-webkit-tests: http://gist.github.com/442392. I asked question about it on [e-mail], but no reply so far.

[Pavel Feldman]

Comment on attachment 58690 [details]
Uncopyable subtitle (.header removed)

Let us try it once more.

[Eric Seidel (no email)]

(In reply to comment #12)
> (In reply to comment #11)
> 
> I wish I could fix this test, but I can't even run it. My full output from run-webkit-tests: http://gist.github.com/442392. I asked question about it on [e-mail], but no reply so far.

Looks like you haven't built webkit.  "build-webkit --release" will build a copy of webkit.  You need a copy of webkit in order for run-webkit-tests to work. :)
http://webkit.org/building/build.html

[WebKit Commit Bot]

Comment on attachment 58690 [details]
Uncopyable subtitle (.header removed)

Rejecting patch 58690 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--ignore-tests', 'compositing', '--quiet']" exit_code: 1
Last 500 characters of output:
e
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Skipped list contained 'compositing/iframes/composited-iframe.html', but no file of that name could be found
Testing 19117 test cases.
inspector/elements-panel-styles.html -> failed

Exiting early after 1 failures. 15407 tests run.
246.27s total testing time

15406 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
14 test cases (<1%) had stderr output

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

[Pavel Feldman]

> Skipped list contained 'compositing/iframes/composited-iframe.html', but no file of that name could be found
> Testing 19117 test cases.
> inspector/elements-panel-styles.html -> failed

Oh, sorry, I overlooked this one - was confused with the composited iframe and forgot I added this beautiful styles sidebar test earlier.

Nikita, are you planning to fix it or should I use your patch locally to fix / rebaseline the test? Here is what you do:

WebKitTools/Scripts/build-webkit --debug
WebKitTools/Scripts/run-webkit-tests --debug LayoutTests/inspector/elements-panel-styles.html

The reason for failure is in LayoutTests/inspector/elements-tests.js:72:

" (" + section.subtitleElement.textContent + ")" 

Test expects to see the location in the .textContent of the subtitle element. See the expected golden output in LayoutTests/inspector/elements-panel-styles-expected.txt (locations in brackets to the first of records). For fixing this, you would need to get the text from your new not-copyable attributes. You could introduce Section.prototype.subtitleAsText in Section.js and use it in test instead of subtitleElement.textContent for convenience.

[Nikita Vasilyev]

Finally, I run the test. Thanks to Eric and Pavel.

(In reply to comment #16)
> > Skipped list contained 'compositing/iframes/composited-iframe.html', but no file of that name could be found
> > Testing 19117 test cases.
> > inspector/elements-panel-styles.html -> failed
> 
> Oh, sorry, I overlooked this one - was confused with the composited iframe and forgot I added this beautiful styles sidebar test earlier.
> 
> Nikita, are you planning to fix it or should I use your patch locally to fix / rebaseline the test?

You do it. I added method subtitleAsText getter next to the subtitle getter. Turns out, it's not accessible within a test, section.subtitleAsText is undefined for some reason. Then I noticed section.subtitle getter undefined too. So, I don't know what's going on, but I'm pretty sure you do.

[Pavel Feldman]

> You do it. I added method subtitleAsText getter next to the subtitle getter. Turns out, it's not accessible within a test, section.subtitleAsText is undefined for some reason. Then I noticed section.subtitle getter undefined too. So, I don't know what's going on, but I'm pretty sure you do.

Hm. I think your changes to the front-end did not get deployed once you fixed the js files. Make sure they are in WebKitBuiold/Debug/WebCore.framework/Resources/inspector.

[Nikita Vasilyev]

Created attachment 59104 [details]
Uncopyable subtitle (test fixed)

(In reply to comment #18)
> Hm. I think your changes to the front-end did not get deployed once you fixed the js files.

Exactly!

[WebKit Commit Bot]

Comment on attachment 59104 [details]
Uncopyable subtitle (test fixed)

Clearing flags on attachment: 59104

Committed r61494: <http://trac.webkit.org/changeset/61494>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.