1467112 - Fix about:checkerboard

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

At some point firefox made changes to CSP and now about:checkerboard doesn't actually show the checkerboard instance details. Bug 1453989 might be related.

Comment 1

[:Gijs (he/him)]

Yeah:

 link.href = "javascript:showReport(" + i + ")";

that won't work anymore. Those should have been updated to be buttons with script-added event listeners from aboutCheckerboard.js . Sorry for not noticing this in review. :-(

Comment 2

[Kartikaya Gupta (email:[e-mail])]

No worries. I have the patch written locally, just waiting on my build to test it.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1467112 - Use listener instead of javascript: URL to conform to CSP.

Review commit: https://reviewboard.mozilla.org/r/249618/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/249618/

Comment 4

[Code Review Bot [:reviewbot]]

Comment on attachment 8983777 [details]
Bug 1467112 - Use listener instead of javascript: URL to conform to CSP.

https://reviewboard.mozilla.org/r/249618/#review255810


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/aboutcheckerboard/content/aboutCheckerboard.js:21
(Diff revision 1)
>    for (var i = 0; i < reports.length; i++) {
>      let text = "Severity " + reports[i].severity + " at " + new Date(reports[i].timestamp).toString();
>      let link = document.createElement("a");
> -    link.href = "javascript:showReport(" + i + ")";
> +    link.href = "#";
> +    let index = i; // for capturing in the lambda
> +    link.addEventListener('click', function() { showReport(index); return false; });

Error: Strings must use doublequote. [eslint: quotes]

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1467112 - Use listener instead of javascript: URL to conform to CSP.

Review commit: https://reviewboard.mozilla.org/r/249626/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/249626/

Comment 6

[:Gijs (he/him)]

Comment on attachment 8983782 [details]
Bug 1467112 - Use listener instead of javascript: URL to conform to CSP.

https://reviewboard.mozilla.org/r/249626/#review255834

::: toolkit/components/aboutcheckerboard/content/aboutCheckerboard.js:20
(Diff revision 1)
> +    let index = i; // for capturing in the lambda
> +    link.addEventListener("click", function() { showReport(index); return false; });

Could we not just define the iteration variable as `let` or does that not work?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Ah, that does work. Will update the patch.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8983782 [details]
Bug 1467112 - Use listener instead of javascript: URL to conform to CSP.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/249626/diff/1-2/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/997adbec4c49
Use listener instead of javascript: URL to conform to CSP. r=Gijs

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/997adbec4c49