Closed Bug 1467112 Opened 2 years ago Closed 2 years ago

Fix about:checkerboard

Categories

(Core :: Panning and Zooming, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: kats, Assigned: kats)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
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. :-(
No worries. I have the patch written locally, just waiting on my build to test it.
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]
Attachment #8983777 - Attachment is obsolete: true
Attachment #8983777 - Flags: review?(gijskruitbosch+bugs)
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?
Attachment #8983782 - Flags: review?(gijskruitbosch+bugs) → review+
Ah, that does work. Will update the patch.
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/997adbec4c49
Use listener instead of javascript: URL to conform to CSP. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/997adbec4c49
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.