Bug 40420 - Web Inspector: Prevent from copying "filename.css" in Styles pane
Summary: Web Inspector: Prevent from copying "filename.css" in Styles pane
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://screenr.com/YMK
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-10 05:32 PDT by Nikita Vasilyev
Modified: 2010-06-19 09:47 PDT (History)
10 users (show)

See Also:


Attachments
Uncopyable subtitle (3.25 KB, patch)
2010-06-10 10:25 PDT, Nikita Vasilyev
pfeldman: review-
Details | Formatted Diff | Diff
Uncopyable subtitle (3.14 KB, patch)
2010-06-11 10:04 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Uncopyable subtitle (.header removed) (3.11 KB, patch)
2010-06-14 12:59 PDT, Nikita Vasilyev
pfeldman: review-
Details | Formatted Diff | Diff
Uncopyable subtitle (test fixed) (5.19 KB, patch)
2010-06-18 06:32 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2010-06-10 05:32:46 PDT
Screencast: http://screenr.com/YMK
Comment 1 Pavel Feldman 2010-06-10 05:53:39 PDT
We should render location using pseudo elements (::after or ::before). Pseudo elements do not get into the clipboard.
Comment 2 Nikita Vasilyev 2010-06-10 06:34:50 PDT
(In reply to comment #1)
How can we change a content of pseudo elements via JavaScript?
Comment 3 Pavel Feldman 2010-06-10 06:44:15 PDT
(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>
Comment 4 Nikita Vasilyev 2010-06-10 10:25:00 PDT
Created attachment 58382 [details]
Uncopyable subtitle
Comment 5 Pavel Feldman 2010-06-11 06:03:15 PDT
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".
Comment 6 Nikita Vasilyev 2010-06-11 09:29:04 PDT
(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.
Comment 7 Nikita Vasilyev 2010-06-11 10:04:51 PDT
Created attachment 58482 [details]
Uncopyable subtitle
Comment 8 Nikita Vasilyev 2010-06-14 10:24:06 PDT
> 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?
Comment 9 Pavel Feldman 2010-06-14 11:54:54 PDT
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!
Comment 10 Nikita Vasilyev 2010-06-14 12:59:40 PDT
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.
Comment 11 WebKit Commit Bot 2010-06-17 00:16:49 PDT
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
Comment 12 Nikita Vasilyev 2010-06-17 10:02:25 PDT
(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 webkit-help@lists.webkit.org, but no reply so far.
Comment 13 Pavel Feldman 2010-06-17 10:20:01 PDT
Comment on attachment 58690 [details]
Uncopyable subtitle (.header removed)

Let us try it once more.
Comment 14 Eric Seidel (no email) 2010-06-17 14:30:53 PDT
(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 webkit-help@lists.webkit.org, 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
Comment 15 WebKit Commit Bot 2010-06-17 16:13:10 PDT
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
Comment 16 Pavel Feldman 2010-06-17 22:57:51 PDT
> 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.
Comment 17 Nikita Vasilyev 2010-06-18 02:46:21 PDT
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.
Comment 18 Pavel Feldman 2010-06-18 05:19:08 PDT
> 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.
Comment 19 Nikita Vasilyev 2010-06-18 06:32:54 PDT
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!
Comment 20 WebKit Commit Bot 2010-06-19 09:47:47 PDT
Comment on attachment 59104 [details]
Uncopyable subtitle (test fixed)

Clearing flags on attachment: 59104

Committed r61494: <http://trac.webkit.org/changeset/61494>
Comment 21 WebKit Commit Bot 2010-06-19 09:47:53 PDT
All reviewed patches have been landed.  Closing bug.