Bug 160774 - Web Inspector: ER: Copy Node as HTML with Computed Style
Summary: Web Inspector: ER: Copy Node as HTML with Computed Style
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-08-11 12:11 PDT by James Craig
Modified: 2016-12-13 15:36 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.19 KB, patch)
2016-08-20 22:40 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (3.21 KB, patch)
2016-08-20 22:43 PDT, Devin Rousso
joepeck: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Craig 2016-08-11 12:11:33 PDT
Web Inspector: ER: Copy Node as HTML with Computed Style

It'd be super useful to have a variant of the "Copy as HTML" feature that dumped each element's computed style properties into the style attribute of each tag. It'd be harder to read the generated HTML, but easier to debug and create test cases.
Comment 1 Radar WebKit Bug Importer 2016-08-11 12:11:51 PDT
<rdar://problem/27809183>
Comment 2 Nikita Vasilyev 2016-08-11 12:14:13 PDT
This would be very useful, indeed. I'd use it myself, I make test cases too.
Comment 3 Nikita Vasilyev 2016-08-17 11:25:11 PDT
Devin, this seems right up your alley. Feel free to assign it to yourself if you want to work on this.
Comment 4 Devin Rousso 2016-08-18 18:56:36 PDT
Do we want to dump the computed style of each element or just the selected/context-menu element?  Also, do we want to display all the computed properties, or just the ones that have been specified/inherited?
Comment 5 Nikita Vasilyev 2016-08-19 14:20:45 PDT
(In reply to comment #4)
> Do we want to dump the computed style of each element or just the
> selected/context-menu element?

Selected/context-menu element and all its descendants. I imagine it can
blow up very easily, but I'd rather deal with this later.

> Also, do we want to display all the computed
> properties, or just the ones that have been specified/inherited?

The ones that have been specified/inherited.
Comment 6 Devin Rousso 2016-08-20 22:40:15 PDT
Created attachment 286554 [details]
Patch

I strayed away from the description slightly due to the fact that we are unable to reliably fetch the computed style of each node (including children) in a synchronous way (at least with the current implementation).  Given that there is already a "Copy as HTML", I thought that it made more sense to just add "Copy Computed Style" that dumps a newline-separated list of the set/inherited CSS properties to the clipboard.
Comment 7 WebKit Commit Bot 2016-08-20 22:41:56 PDT
Attachment 286554 [details] did not pass style-queue:


ERROR: Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:711:  Line contains single-quote character.  [js/syntax] [5]
ERROR: Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:712:  Line contains single-quote character.  [js/syntax] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Devin Rousso 2016-08-20 22:43:34 PDT
Created attachment 286555 [details]
Patch
Comment 9 Nikita Vasilyev 2016-08-21 14:28:00 PDT
(In reply to comment #6)
> Created attachment 286554 [details]
> Patch
> 
> I strayed away from the description slightly due to the fact that we are
> unable to reliably fetch the computed style of each node (including
> children) in a synchronous way (at least with the current implementation). 
> Given that there is already a "Copy as HTML", I thought that it made more
> sense to just add "Copy Computed Style" that dumps a newline-separated list
> of the set/inherited CSS properties to the clipboard.

I don't think this is very helpful. Getting computed styled of all descendants is a harder task, but that's what would make this feature useful.
Comment 10 Devin Rousso 2016-08-21 15:15:02 PDT
(In reply to comment #9)
> I don't think this is very helpful. Getting computed styled of all
> descendants is a harder task, but that's what would make this feature useful.

I would agree that getting the computed style of all descendants would be more useful, but the issue is with fetching those styles.  Since we lazy-load instances of DOMNodeStyles when the user selects an element in the DOM tree, if they haven't selected a descendent node and try to dump the computed styles via the context menu item on a parent, it will just show an empty CSSStyleDeclaration (no properties).

I am pretty sure that I could include a WrappedPromise in some of the functions (specifically DOMNodeStyles.prototype.refresh) that would let us tie into the fetch when it completes, but this has the possibility of lagging and may add items to the clipboard in a very asynchronous manner.

If we are OK with that, then I can do this, but I am worried about situations where the user attempts to click the context menu item on <body> and has the entire DOM tree copied to the clipboard after a few seconds since it has to perform so many calls to generate the computed style of each descendant.
Comment 11 Joseph Pecoraro 2016-08-22 13:47:10 PDT
Comment on attachment 286555 [details]
Patch

r- while we discuss implementing the more useful feature.
Comment 12 Joseph Pecoraro 2016-08-22 14:10:21 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > I don't think this is very helpful. Getting computed styled of all
> > descendants is a harder task, but that's what would make this feature useful.
> 
> I would agree that getting the computed style of all descendants would be
> more useful, but the issue is with fetching those styles.  Since we
> lazy-load instances of DOMNodeStyles when the user selects an element in the
> DOM tree, if they haven't selected a descendent node and try to dump the
> computed styles via the context menu item on a parent, it will just show an
> empty CSSStyleDeclaration (no properties).

I think the more useful feature is what users will want and expect.

The question then is how do we implement it? You've alluded to how the frontend could do this, laboriously, by requesting information about every node. Which breaks down for large documents. But we haven't explored other options.

I think this could be done in the backend.

Take for example, this test page, and the user wants to copy the styles for the <body> element:

> index.html:
> 
>     <!DOCTYPE html>
>     <html>
>     <head>
>         <link rel="stylesheet" href="styles1.css">
>         <link rel="stylesheet" href="styles2.css">
>         <script>
>         window.addEventListener("load", function() {
>             document.getElementById("red").style.color = "red";
>         });
>         </script>
>     </head>
>     <body>
>         <div class="foo">
>             <p style="color:green">Green</p>
>             <p>Blue</p>
>             <p id="red">Red</p>
>         </div>
>     </body>
>     </html>
> 
> styles1.css:
> 
>     .foo { color: blue; text-decoration: underline; }
>     .foo:hover { color: black; }
>
> styles2.css:
>
>     body { margin: 0; padding: 0; }


There are a few approaches I can think of.

1. Iterate all nodes from the target, producing inline style for non-inherited overridden styles:

    <body style="margin: 0; padding: 0">
        <div class="foo" style="color: blue">
            <p style="color: green; text-decoration: underline">Green</p>
            <p style="color: blue; text-decoration: underline">Blue</p>
            <p id="red" style="color: red; text-decoration: underline">Red</p>
        </div>
    </body>

2. Collect the stylesheets into an inline <style> alongside the nodes iterated from target with their inline styles that don't come from a stylesheet.

    <style>
    .foo { color: blue; text-decoration: underline; }
    .foo:hover { color: black; }
    body { margin: 0; padding: 0; }
    </style>
    <body>
        <div class="foo">
            <p style="color: green">Green</p>
            <p>Blue</p>
            <p id="red" style="color: red">Red</p>
        </div>
    </body>

Forgive any subtle typos or logic errors I may have made, I typed this all into a bugzilla comment without testing.

There are pro and cons of both approaches as I see it:

    1. Pure Inline Styles
        - standalone, easy to copy/paste
        - verbose, numerous styles may be repeated over and over again
        - a pure snapshot, missing pseudo states (like :hover)
        - can provide developer with surprising discovery ("oh this style is on a lot of elements, why?")

    2. Stylesheet + Inline Styles
        - can contain "unused styles" - but that can be pseudo states (like :hover)
        - total verbose, copying entire stylesheets can be wasteful
        - more readable DOM, no unnecessary inline style clutter
        - less copy/pasteable, can be a good thing

I rather like (2) because of the fact it can handle pseudo classes like :hover. But I'd like to hear what other users think.