Created attachment 363627[details]
Archive of layout-test-results from ews102 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 363631[details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Created attachment 363633[details]
Archive of layout-test-results from ews112 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 363635[details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
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 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!
Created attachment 363863[details]
Archive of layout-test-results from ews100 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 363865[details]
Archive of layout-test-results from ews112 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 363907[details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Created attachment 364043[details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
2019-03-05 01:52 PST, Devin Rousso
2019-03-05 02:54 PST, EWS Watchlist
2019-03-05 03:12 PST, EWS Watchlist
2019-03-05 03:23 PST, EWS Watchlist
2019-03-05 04:08 PST, EWS Watchlist
2019-03-05 11:56 PST, Devin Rousso
2019-03-05 12:09 PST, Devin Rousso
2019-03-06 12:11 PST, Devin Rousso
2019-03-07 01:20 PST, Devin Rousso
2019-03-07 02:23 PST, EWS Watchlist
2019-03-07 03:06 PST, EWS Watchlist
2019-03-07 10:25 PST, Devin Rousso
2019-03-07 11:42 PST, EWS Watchlist
2019-03-08 10:18 PST, Devin Rousso
2019-03-08 10:21 PST, Devin Rousso
2019-03-08 11:39 PST, EWS Watchlist