Skip to content

fix(worker)!: remove dead highlight-load-lang code path#5198

Merged
marcoscaceres merged 10 commits into
mainfrom
alert-autofix-24
Apr 19, 2026
Merged

fix(worker)!: remove dead highlight-load-lang code path#5198
marcoscaceres merged 10 commits into
mainfrom
alert-autofix-24

Conversation

@marcoscaceres

@marcoscaceres marcoscaceres commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Removes the unused highlight-load-lang action from the worker
  • This code path has been dead since the highlight system was refactored (see Example 134: Load custom syntax highlighting language seems obselete #4934)
  • The Copilot Autofix that tried to add URL origin validation was broken in blob workers (self.location.origin is "null"), so removing the dead code is the correct fix

Test plan

  • Modified Web platform tests — no observable behavior change (dead code removal)

@marcoscaceres

Copy link
Copy Markdown
Contributor Author

@copilot see the CI error about the commit message. Can you fix that please?

Copilot AI commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

@copilot see the CI error about the commit message. Can you fix that please?

Done in commit 9faf188. The CI check (action-semantic-pull-request) validates the PR title, not the individual commit messages. The new commit uses fix(worker): validate URL origin before importScripts in highlight-load-lang as the message. The PR title will also need to be updated to a conventional commit format (e.g., fix(worker): validate URL origin before importScripts in highlight-load-lang) for the CI check to pass.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh gh auth status (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@marcoscaceres marcoscaceres changed the title Potential fix for code scanning alert no. 24: Client-side URL redirect fix: code scanning alert no. 24: Client-side URL redirect Apr 14, 2026
@marcoscaceres marcoscaceres changed the title fix: code scanning alert no. 24: Client-side URL redirect fix(worker): validate URL origin before importScripts Apr 14, 2026
@marcoscaceres

Copy link
Copy Markdown
Contributor Author

@copilot Can you fix the lint error from ci?

Copilot AI commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

@copilot Can you fix the lint error from ci?

Fixed in commit d7c435c. The two Prettier formatting issues were long lines that needed to be broken up — the multi-condition if statement and the long error message string.

@marcoscaceres marcoscaceres marked this pull request as ready for review April 15, 2026 09:02
marcoscaceres and others added 4 commits April 17, 2026 17:55
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ad-lang

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
Agent-Logs-Url: https://github.com/speced/respec/sessions/eaf97df3-cfd4-4999-92cf-047f0b3b3ca4

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
The highlight-load-lang action is unused by production code (see #4934).
The same-origin check added by Copilot Autofix was broken in blob
workers (self.location.origin is "null"), silently breaking the only
remaining consumer. Remove the dead code instead of hardening it.

Copilot AI 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.

Pull request overview

This PR updates the ReSpec web worker script with the goal of addressing a security finding around importScripts() usage in the highlighting worker.

Changes:

  • Bumps the worker version header comment to v1.1.0.
  • Simplifies the worker message handler to only process the "highlight" action (removing the "highlight-load-lang" branch).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread worker/respec-worker.js Outdated
Comment thread worker/respec-worker.js Outdated
@marcoscaceres marcoscaceres changed the title fix(worker): validate URL origin before importScripts fix(worker): remove dead highlight-load-lang code path Apr 17, 2026
@marcoscaceres

Copy link
Copy Markdown
Contributor Author

@copilot can you check and fix the ci issues?

Agent-Logs-Url: https://github.com/speced/respec/sessions/78a14774-8331-43c0-b95e-83e38a6bd570

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>

Copilot AI commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

@copilot can you check and fix the ci issues?

Fixed in commit 54686e5. The Karma unit test "highlights remote languages not bundled by default with ReSpec" was failing because it exercised the highlight-load-lang handler that was intentionally removed. I removed the failing test along with its now-unused fixtures (tests/spec/core/highlight.html and tests/support-files/hljs-testlang.js).

@marcoscaceres

Copy link
Copy Markdown
Contributor Author

@copilot rebase

# Conflicts:
#	tests/spec/core/highlight.html
#	worker/respec-worker.js

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>

Copilot AI commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

@copilot rebase

Done in commit 645ac27. The branch is now merged with the latest main (f351bbb). I resolved the conflicts by keeping our dead-code removal in worker/respec-worker.js and the test fixture deletion.

@marcoscaceres

Copy link
Copy Markdown
Contributor Author

Merging this as a hot fix, as it doesn't touch any code, but addresses the CI issue.

@marcoscaceres

Copy link
Copy Markdown
Contributor Author

oh, no, wait!... it does touch code. I misread it on my phone.

… optional chain

- Remove Object.assign clone (structured clone already provides fresh data)
- Early return for non-highlight actions (don't echo unknown messages)
- Add optional chain on data.languages for robustness

Copilot AI 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.

Pull request overview

This PR removes a long-dead worker message/action (highlight-load-lang) from ReSpec’s syntax-highlighting worker, along with the associated test fixture that exercised that path.

Changes:

  • Remove the "highlight-load-lang" message handler from worker/respec-worker.js, leaving the worker to handle only "highlight" requests.
  • Delete the test HTML/fixture JS that posted "highlight-load-lang" to the worker.
  • Drop the spec test that asserted remote/custom language loading via that removed action.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
worker/respec-worker.js Deletes the dead "highlight-load-lang" branch and simplifies the message handler to only process "highlight".
tests/support-files/hljs-testlang.js Removes the now-unused custom highlight language fixture.
tests/spec/core/highlight.html Removes the spec fixture document that posted "highlight-load-lang" to the worker.
tests/spec/core/highlight-spec.js Removes the spec test that depended on "highlight-load-lang" to register a remote language.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@marcoscaceres

Copy link
Copy Markdown
Contributor Author

@sidvishnoi this one is high priority — it removes the flaky CDN highlight test that's failing CI on 4+ other PRs. Clean dead code removal (per #4934). Would appreciate a quick look when you get a chance.

@sidvishnoi sidvishnoi changed the title fix(worker): remove dead highlight-load-lang code path fix(worker)!: remove dead highlight-load-lang code path Apr 19, 2026

@sidvishnoi sidvishnoi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Marked as breaking change (!) in PR title.

@marcoscaceres marcoscaceres merged commit 58b1303 into main Apr 19, 2026
9 checks passed
@marcoscaceres marcoscaceres deleted the alert-autofix-24 branch April 19, 2026 12:27
Copilot AI added a commit that referenced this pull request Apr 19, 2026
Merge latest main which removes dead highlight-load-lang code path
(#5198). The highlight.html fixture that our branch updated to use
document.respec.worker has been deleted on main as dead code.

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants