Skip to content

[dev] Remove many inlined style attributes#5908

Merged
StephenHulme merged 6 commits into
developfrom
sh51/csp-remove-inline-style-attr
Jul 9, 2026
Merged

[dev] Remove many inlined style attributes#5908
StephenHulme merged 6 commits into
developfrom
sh51/csp-remove-inline-style-attr

Conversation

@StephenHulme

@StephenHulme StephenHulme commented Jul 8, 2026

Copy link
Copy Markdown
Member

Removes many usages - but not all - of inline style attributes, replacing them with the Bootstrap 4 equivalents.

Based on #5907 and a stepping stone to improved application security - see https://centralcsp.com/docs/style-src-attr

Changes proposed in this pull request

  • Replace variations on style='display: none' with class="d-none"
  • Replace variations on style='text-align: center' with class="text-center"
  • Replace variations on style="background:red" with custom .bg-red style
  • Replace more complex inline styles with bootstrap equivalents where appropiate

Instructions for Reviewers

[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to main]
    - Check story numbers included
    - Check for debug code
    - Check version

Run:
grep -rI --include=\*.html\* --exclude=\*.svg 'style=' app/ > style_src_attr.txt
@StephenHulme StephenHulme requested a review from BenTopping July 8, 2026 15:11
@StephenHulme StephenHulme self-assigned this Jul 8, 2026
@StephenHulme StephenHulme changed the base branch from develop to sh51/nonce-security-blob-2 July 8, 2026 15:12

@BenTopping BenTopping left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice - definitely good to move to one consistent way of styling.
Are there are lot of remaining inline-styles to change? I assume there are some trickier ones left?

Comment thread app/frontend/stylesheets/all/sequencescape.scss
@StephenHulme

Copy link
Copy Markdown
Member Author

There are some trickier ones left:

app/views/shared/_progress_statistics_summary.html.erb:               style="width: <%= percentage %>%"
app/views/samples/_component_samples.html.erb:  <div style="margin-top: 25px;">
app/views/samples/_accession_statuses.html.erb:  <div style="max-height: 35vh; overflow-y: auto;">
app/views/plates_from_tubes/new.html.erb:        <div class="alert alert-danger d-none" id="duplicate_warning" style="width: 72%">

In-lining widths is probably the trickiest: style="width: <%= percentage %>%"

A larger problem is old jQuery which just doesn't understand the concept of CSP, there are now updates to the jQuery library which can handle this smoothly. But that's a challenge for another PR.

The bootstrap equivalent class="d-none" overrides the display:block that
is used as part of loading interactivity.
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.83%. Comparing base (3cc3465) to head (7b8c1a8).

Additional details and impacted files
@@                     Coverage Diff                     @@
##           sh51/nonce-security-blob-2    #5908   +/-   ##
===========================================================
  Coverage                       84.82%   84.83%           
===========================================================
  Files                            1492     1492           
  Lines                           33936    33936           
  Branches                         3613     3613           
===========================================================
+ Hits                            28787    28788    +1     
+ Misses                           4302     4299    -3     
- Partials                          847      849    +2     
Flag Coverage Δ
javascript 76.31% <ø> (ø)
ruby 84.71% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from sh51/nonce-security-blob-2 to develop July 9, 2026 11:28
@StephenHulme StephenHulme merged commit 6d117d5 into develop Jul 9, 2026
16 checks passed
@StephenHulme StephenHulme deleted the sh51/csp-remove-inline-style-attr branch July 9, 2026 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants