Bug 195311 - Web Inspector: Canvas: export recording as HTML
Summary: Web Inspector: Canvas: export recording as HTML
Status: RESOLVED FIXED
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: WebInspectorCanvasRecording WebInspectorCanvasTab
  Show dependency treegraph
 
Reported: 2019-03-04 20:41 PST by Devin Rousso
Modified: 2019-03-12 11:59 PDT (History)
7 users (show)

See Also:


Attachments
Patch (35.81 KB, patch)
2019-03-05 01:52 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-highsierra (2.46 MB, application/zip)
2019-03-05 02:54 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews104 for mac-highsierra-wk2 (2.83 MB, application/zip)
2019-03-05 03:12 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews112 for mac-highsierra (2.38 MB, application/zip)
2019-03-05 03:23 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (2.96 MB, application/zip)
2019-03-05 04:08 PST, EWS Watchlist
no flags Details
Patch (42.13 KB, patch)
2019-03-05 11:56 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (42.89 KB, patch)
2019-03-05 12:09 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
[HTML] Output after Patch is applied (1.04 MB, text/html)
2019-03-06 12:11 PST, Devin Rousso
no flags Details
Patch (43.57 KB, patch)
2019-03-07 01:20 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-highsierra (2.44 MB, application/zip)
2019-03-07 02:23 PST, EWS Watchlist
no flags Details
Archive of layout-test-results from ews112 for mac-highsierra (2.27 MB, application/zip)
2019-03-07 03:06 PST, EWS Watchlist
no flags Details
Patch (43.22 KB, patch)
2019-03-07 10:25 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (2.57 MB, application/zip)
2019-03-07 11:42 PST, EWS Watchlist
no flags Details
Patch (44.76 KB, patch)
2019-03-08 10:18 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (45.06 KB, patch)
2019-03-08 10:21 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (2.77 MB, application/zip)
2019-03-08 11:39 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-03-04 20:41:14 PST
This would be incredibly useful as a reduction "generator".
Comment 1 Radar WebKit Bug Importer 2019-03-04 20:41:41 PST
<rdar://problem/48588673>
Comment 2 Devin Rousso 2019-03-05 01:52:23 PST
Created attachment 363623 [details]
Patch
Comment 3 EWS Watchlist 2019-03-05 02:54:49 PST Comment hidden (obsolete)
Comment 4 EWS Watchlist 2019-03-05 02:54:51 PST Comment hidden (obsolete)
Comment 5 EWS Watchlist 2019-03-05 03:12:43 PST Comment hidden (obsolete)
Comment 6 EWS Watchlist 2019-03-05 03:12:45 PST Comment hidden (obsolete)
Comment 7 EWS Watchlist 2019-03-05 03:23:09 PST Comment hidden (obsolete)
Comment 8 EWS Watchlist 2019-03-05 03:23:11 PST Comment hidden (obsolete)
Comment 9 EWS Watchlist 2019-03-05 04:08:55 PST Comment hidden (obsolete)
Comment 10 EWS Watchlist 2019-03-05 04:08:56 PST Comment hidden (obsolete)
Comment 11 Devin Rousso 2019-03-05 11:56:09 PST
Created attachment 363664 [details]
Patch
Comment 12 Devin Rousso 2019-03-05 12:09:49 PST
Created attachment 363668 [details]
Patch
Comment 13 Devin Rousso 2019-03-06 12:11:21 PST
Created attachment 363770 [details]
[HTML] Output after Patch is applied

This is an example export from recording <https://devinrousso.com> for about ~200 frames.
Comment 14 Joseph Pecoraro 2019-03-06 22:16:44 PST
Comment on attachment 363668 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363668&action=review

This is awesome!

r=me

> Source/WebInspectorUI/UserInterface/Models/Recording.js:498
> +            return JSON.stringify(value);

Nice! This will protect against injection!

> Source/WebInspectorUI/UserInterface/Models/Recording.js:514
> +        lines.push(`<!DOCTYPE html>`);
> +        lines.push(`<body>`);
> +        lines.push(`<style>`);
> +        lines.push(`    body {`);
> +        lines.push(`        margin: 0;`);
> +        lines.push(`    }`);
> +        lines.push(`    canvas {`);
> +        lines.push(`        max-width: calc(100% - 40px);`);
> +        lines.push(`        max-height: calc(100% - 40px);`);
> +        lines.push(`        padding: 20px;`);
> +        lines.push(`    }`);
> +        lines.push(`</style>`);
> +        lines.push(`<script>`);
> +        lines.push(`"use strict";`);

Template strings allow newlines, so this could be cleaned up a bit, that said I like when things line up nicely.

Lets also make an effort to make this more standard and include a nice title for browser tabs. Maybe:

    <html>
    <head>
        <title>${this._displayName}</title>
    </head>
    <body>
    ...

Though you'd want to escape the displayName if it can be user defined to avoid script injection.

For example if someone did `console.record(ctx, {name: "</title><script>alert(1);</script>});`

> Source/WebInspectorUI/UserInterface/Models/Recording.js:546
> +                if (name === "setPath" || name === "currentX" || name === "currentY")
> +                    continue;

"getPath" is also an InspectorAddition that may need to be skipped.

> Source/WebInspectorUI/UserInterface/Models/Recording.js:566
> +        lines.push(`            console.record(context, {name: "${this._displayName}"});`);

If the displayName is user defined you should probably protect against script injection. You could JSON.stringify it here:

    lines.push(` console.record(context, {name: ${JSON.stringify(this._displayName)});`);

For example if someone did `console.record(ctx, {name: "\"}); alert(1); ({foo:\");`.

> Source/WebInspectorUI/UserInterface/Models/Recording.js:584
> +                if (!action.valid)
> +                    contextString = `// ` + contextString;

Clever! This could move above the call string, next to the other contextString setup.

> Source/WebInspectorUI/UserInterface/Models/Recording.js:656
> +                lines.push(`            createImageBitmap(image).then(function (imageBitmap) {`);

Style: It is unusual to have the space after `function` here.

> Source/WebInspectorUI/UserInterface/Models/Recording.js:702
> +                    lines.push(`rebuildDOMMatrix(${index}, ${JSON.stringify(data)});`)

How about just passing this as an array:

    let data = [object.a, object.b, object.c, object.d, object.e, object.f];

Then rebuildDOMMatrix just takes the data:

    function rebuildDOMMatrix(key, data) {
        objects[key] = new DOMMatrix(data)
    }

Also interesting, DOMMatrix has a toString() which the constructor takes, so you could alternatively do:

    let data = object.toString();

But I like the array version better.
Comment 15 Joseph Pecoraro 2019-03-06 22:19:53 PST
Comment on attachment 363668 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363668&action=review

>> Source/WebInspectorUI/UserInterface/Models/Recording.js:702
>> +                    lines.push(`rebuildDOMMatrix(${index}, ${JSON.stringify(data)});`)
> 
> How about just passing this as an array:
> 
>     let data = [object.a, object.b, object.c, object.d, object.e, object.f];
> 
> Then rebuildDOMMatrix just takes the data:
> 
>     function rebuildDOMMatrix(key, data) {
>         objects[key] = new DOMMatrix(data)
>     }
> 
> Also interesting, DOMMatrix has a toString() which the constructor takes, so you could alternatively do:
> 
>     let data = object.toString();
> 
> But I like the array version better.

Hmm, actually using DOMMatrix's toString would support a 3d matrix. Might be worth doing it that way then!
Comment 16 Devin Rousso 2019-03-07 01:20:57 PST
Created attachment 363855 [details]
Patch
Comment 17 EWS Watchlist 2019-03-07 02:23:02 PST Comment hidden (obsolete)
Comment 18 EWS Watchlist 2019-03-07 02:23:04 PST Comment hidden (obsolete)
Comment 19 EWS Watchlist 2019-03-07 03:06:02 PST Comment hidden (obsolete)
Comment 20 EWS Watchlist 2019-03-07 03:06:03 PST Comment hidden (obsolete)
Comment 21 Devin Rousso 2019-03-07 10:25:45 PST
Created attachment 363892 [details]
Patch
Comment 22 EWS Watchlist 2019-03-07 11:42:25 PST Comment hidden (obsolete)
Comment 23 EWS Watchlist 2019-03-07 11:42:26 PST Comment hidden (obsolete)
Comment 24 Devin Rousso 2019-03-08 10:18:48 PST
Created attachment 364027 [details]
Patch
Comment 25 Devin Rousso 2019-03-08 10:21:08 PST
Created attachment 364029 [details]
Patch
Comment 26 EWS Watchlist 2019-03-08 11:39:30 PST Comment hidden (obsolete)
Comment 27 EWS Watchlist 2019-03-08 11:39:33 PST Comment hidden (obsolete)
Comment 28 WebKit Commit Bot 2019-03-12 11:59:36 PDT
Comment on attachment 364029 [details]
Patch

Clearing flags on attachment: 364029

Committed r242809: <https://trac.webkit.org/changeset/242809>
Comment 29 WebKit Commit Bot 2019-03-12 11:59:38 PDT
All reviewed patches have been landed.  Closing bug.