Remove unsave inline scripts and styles

[xi]

Fixes #1248

As a first step I pushed a test. It works locally but I was not sure whether it would work with CI, so I pushed it. I hope that is OK. For some reason CI does not run so I have no clue whether the test I added actually works.

[tim-schilling]

Thank you for the PR. My comments vary between discussions and change requests.

• I'd like to see the scripts moved to the panel classes.
• I also feel pretty strongly about moving the check to the Makefile.
• The comment regarding the applyStyle approach is more of a discussion.

[tim-schilling]

I think it's more appropriate for this check of inline styles or scripts to exist in the Makefile, then including it here with the other static checks run by tox.

Doing so will also make it easier for developers to run this check before pushing to github and having CI catch it.

[tim-schilling]

I think I'd be more in favor of an approach that would find all elements that have a class such as djdt-dynamic-style, then have it apply the styles listed in the data attribute. That way the javascript doesn't need to know anything about the specific styles that the HTML elements are having applied.

[xi]

Thanks for the quick feedback. I had tried to simply revert 87364f7 which I now realize was a bit naive.

I would like to avoid making applyStyles() more generic. It is just meant to be compatibility code until a better solution is found. New code should ideally not use it IMHO.

[codecov]

Codecov Report

Merging #1445 (9025ab6) into master (0b58338) will decrease coverage by 0.87%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1445      +/-   ##
==========================================
- Coverage   87.68%   86.80%   -0.88%     
==========================================
  Files          29       29              
  Lines        1591     1591              
  Branches      224      224              
==========================================
- Hits         1395     1381      -14     
- Misses        145      153       +8     
- Partials       51       57       +6     

Impacted Files	Coverage Δ
debug_toolbar/panels/history/panel.py	89.58% <0.00%> (-6.25%)	⬇️
debug_toolbar/panels/templates/views.py	62.79% <0.00%> (-4.66%)	⬇️
debug_toolbar/panels/signals.py	81.81% <0.00%> (-4.55%)	⬇️
debug_toolbar/panels/headers.py	95.45% <0.00%> (-4.55%)	⬇️
debug_toolbar/toolbar.py	88.75% <0.00%> (-2.50%)	⬇️
debug_toolbar/panels/cache.py	82.26% <0.00%> (-2.13%)	⬇️
debug_toolbar/middleware.py	95.00% <0.00%> (-1.67%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b58338...905e572. Read the comment docs.

[tim-schilling]

@xi I squashed your commits and converted the applyStyles function to something I'd prefer to maintain. I understand your concern regarding "It is just meant to be compatibility code until a better solution is found.", but I don't think there's a great solution out there. I find this approach more flexible than having to call applyStyles per dynamic style desired.

[matthiask]

I'm afraid I don't understand how this works. Does element.style["background-color"] really set the background color of an element? Shouldn't this be element.style["backgroundColor"]? I did a quick local test and it seems to work but I'm surprised that it does.

(applyStyles is ugly, yes, but I don't have a better idea.)

[tim-schilling]

@matthiask There's a brief mention of using the dashed names here in the docs of CSSStyleDeclaration. However, you're right, it's probably best to avoid using the dashed names.

[tim-schilling]

I updated the applyStyles function to mention the expected format and switched background-color to backgroundColor.

[xi]

For completeness' sake:

Setting the dashed attribute attribute must invoke setProperty() with the first argument being dashed attribute, as second argument the given value, and no third argument. Any exceptions thrown must be re-thrown.
https://www.w3.org/TR/2016/WD-cssom-1-20160317/#the-cssstyledeclaration-interface

So apperantly element.style["background-color"] is fine. That spec is a draft though, so no clue how widely this is supported.

[tim-schilling]

I missed padding-left. This should be good to go now.

[matthiask]

Thank you! 🎉