Bug 40420 - Web Inspector: Prevent from copying "filename.css" in Styles pane
: Web Inspector: Prevent from copying "filename.css" in Styles pane
Status: RESOLVED FIXED
: WebKit
Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
: http://screenr.com/YMK
:
:
:
  Show dependency treegraph
 
Reported: 2010-06-10 05:32 PST by
Modified: 2010-06-19 09:47 PST (History)


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


Note

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


Description From 2010-06-10 05:32:46 PST
Screencast: http://screenr.com/YMK
------- Comment #1 From 2010-06-10 05:53:39 PST -------
We should render location using pseudo elements (::after or ::before). Pseudo elements do not get into the clipboard.
------- Comment #2 From 2010-06-10 06:34:50 PST -------
(In reply to comment #1)
How can we change a content of pseudo elements via JavaScript?
------- Comment #3 From 2010-06-10 06:44:15 PST -------
(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 From 2010-06-10 10:25:00 PST -------
Created an attachment (id=58382) [details]
Uncopyable subtitle
------- Comment #5 From 2010-06-11 06:03:15 PST -------
(From update of attachment 58382 [details])
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 From 2010-06-11 09:29:04 PST -------
(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 From 2010-06-11 10:04:51 PST -------
Created an attachment (id=58482) [details]
Uncopyable subtitle
------- Comment #8 From 2010-06-14 10:24:06 PST -------
> 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 From 2010-06-14 11:54:54 PST -------
(From update of attachment 58482 [details])
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 From 2010-06-14 12:59:40 PST -------
Created an attachment (id=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 From 2010-06-17 00:16:49 PST -------
(From update of attachment 58690 [details])
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 From 2010-06-17 10:02:25 PST -------
(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 From 2010-06-17 10:20:01 PST -------
(From update of attachment 58690 [details])
Let us try it once more.
------- Comment #14 From 2010-06-17 14:30:53 PST -------
(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 From 2010-06-17 16:13:10 PST -------
(From update of attachment 58690 [details])
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 From 2010-06-17 22:57:51 PST -------
> 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 From 2010-06-18 02:46:21 PST -------
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 From 2010-06-18 05:19:08 PST -------
> 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 From 2010-06-18 06:32:54 PST -------
Created an attachment (id=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 From 2010-06-19 09:47:47 PST -------
(From update of attachment 59104 [details])
Clearing flags on attachment: 59104

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