Repository navigation
Conversation
Brings @internetarchive/reviews 1.0.8 across as ia-reviews, ia-review and ia-review-form, with a demo story and the utils and icons they need. Adapted to this repo: tests moved to vitest and co-located, customElement from @src/util/custom-element like the other 28 components, classes on the IA prefix, and fetch-handler in place of the deprecated fetch-handler-service. The a11y assertions run on axe-core directly, since vitest-axe is node-only and the suite runs in real Chromium. Fixes carried in the move: - A review body could inject attributes onto the rendered link. The URL pattern allowed a quote, the match is interpolated into an anchor and rendered with unsafeHTML, and DOMPurify runs before that, so `archive.org/x"onmouseover="alert(1)` became a live handler. Quotes and angle brackets now end a match, and the href is escaped. - Delete reported success on any response, because fetch resolves on a 500. - Delete parameters went unencoded into a query string, so a screenname with a space or an ampersand corrupted the request. - sanitizeReviewBody registered a DOMPurify hook per call on the shared singleton, stacking up one per review and reaching other callers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyEq51DigmsmwGsa7fEqWS
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
- Coverage 90.67% 90.19% -0.49%
==========================================
Files 96 105 +9
Lines 3519 3846 +327
Branches 843 949 +106
==========================================
+ Hits 3191 3469 +278
- Misses 137 158 +21
- Partials 191 219 +28 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Matches the newer components here, which keep glyphs as standalone assets rather than inline lit templates. The class each svg carried moves onto the img that renders it, so the existing CSS and test selectors still find them. Drops add-icon and reviews-icon, which nothing imported, and fixes star-basic's fill, which was "2C2C2C" with no leading hash and so rendered black rather than grey. An svg with no intrinsic size renders 300x150 inside an img, so the two sizes that were previously implicit are pinned by tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyEq51DigmsmwGsa7fEqWS
The component only offers its own "write a review" link when an item has no reviews yet, so with the demo's three reviews the form was unreachable. A host page supplies its own button and sets displayReviewForm on the element; the story now does the same and the usage notes say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyEq51DigmsmwGsa7fEqWS
ia-review truncates a body past 150 characters and offers More/Less, but every body in the story sat under that, so the demo never showed it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyEq51DigmsmwGsa7fEqWS
The demo submitted new reviews as "Ada Fielding", who already has one in the list, so a review you just wrote looked like it had been attributed to someone else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyEq51DigmsmwGsa7fEqWS
Both whitespace patterns were character classes with a pipe inside, so a pipe in a review body counted as whitespace: "Rated 8|10 overall" came out as "Rated 8 10 overall". The classes now list only real whitespace, and the newline pattern groups \r\n instead of spreading it across a class. Reported by GitHub code quality on #110, which flagged the duplicate \n in the second class. The stray pipe was the part that showed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyEq51DigmsmwGsa7fEqWS
The submit button's spinner came from @internetarchive/ia-activity-indicator. ia-status-indicator covers it in loading mode, so the form uses that and the external dependency goes. hideDots gives the bare ring the button wants, and the colour and size come through the theme variables rather than the old component's own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyEq51DigmsmwGsa7fEqWS
The four buttons were native elements wearing ia-styles' .ia-button classes. They're ia-button now: mode="link" for the two in the messages, secondary for Cancel, primary for Submit. ia-button renders its own loading state, so the submit spinner and the ia-status-indicator wiring behind it go away, and @internetarchive/ia-styles is no longer a dependency. That was the last package here built against lit 2, so the CSSResult casts go with it. The link buttons sit inside a sentence, so they reach through ::part(button) to undo the flex display that would otherwise break the line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyEq51DigmsmwGsa7fEqWS
…eviews * origin/main: 1.1.1 (#118) WEBDEV-9132: Fix Safari image-viewer wide-layout breakpoint (#115) 1.1.0 (#116) WEBDEV-9129: Exclude story files from coverage reporting (#113) WEBDEV-9121: Migrate histogram-date-range into elements (#112) WEBDEV-9130: Exclude nested dist and worktree tests from local runs (#114) v1.0.0 (#111) # Conflicts: # package.json # pnpm-lock.yaml
Collaborator
|
Reviewed against the actual pre-migration source, not just the PR description — pulled
Migration itself matches the established pattern from the item-nav and status-indicator moves ( Typecheck, lint, and 124/124 reviews tests pass locally on this branch post-merge with main. No blocking issues. Approving. Unrelated side note, not a blocker: the published — |
jbuckner
added this pull request to stack #123
September 30, 2026 18:25
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgyoVnVd6Wt6HHbGy3Vuvh
Delete calls the global fetch, so the demo's stub fetchHandler never saw it and a click posted to archive.org and failed on CORS. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgyoVnVd6Wt6HHbGy3Vuvh
* origin/main: 1.1.6 (#145) WEBDEV-9329: Update the Firefox add-on link in ia-topnav (#143) WEBDEV-9241: Add ia-wayback-search-form for the home page Wayback block (#134) WEBDEV-9219: Localize the topnav strings (#125) WEBDEV-9051: Migrate the donation form models, currency validator and form section into elements (#98) WEBDEV-9038: Migrate the topnav into elements (#95)
* origin/main: WEBDEV-9052: Migrate edit-donation into elements (#100)
Collaborator
Author
|
Merged main in (and up the stack to #122 and #133). Typecheck, lint and tests pass. Re-ran the demo QA on this preview in Chromium, WebKit and iPhone WebKit, all steps pass. Isa, the qa-failed label is from the CORS error on 9/29, which was the 9114 review app and not this PR. Can you take another look? |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings
@internetarchive/reviews1.0.8 across asia-reviews,ia-reviewandia-review-form, plus a demo story. Tests moved to vitest and co-located,customElementfrom@src/util/custom-elementlike the other 28 components, classes on theIAprefix, andfetch-handlerinstead of the deprecatedfetch-handler-service. The a11y assertions run onaxe-coredirectly because vitest-axe is node-only and the suite runs in real Chromium.Four fixes came along with it, all pre-existing in the published package:
unsafeHTML, and DOMPurify runs before that.archive.org/x"onmouseover="alert(1)in a review body produced a live event handler, confirmed in the browser. Quotes and angle brackets now end a match and the href is escaped. Regression tests at the util and rendered levels.fetchresolves on a 500.&corrupted the request.sanitizeReviewBodyadded a DOMPurify hook per call on the shared singleton, stacking one per review and leaking onto other callers.Still open: the CSRF token sits in the delete query string. That needs petabox to accept it elsewhere first, tracked in WEBDEV-9108.
818 tests pass (121 of them reviews), typecheck, eslint and prettier clean.
WEBDEV-9107: https://webarchive.jira.com/browse/WEBDEV-9107
🤖 Generated with Claude Code
https://claude.ai/code/session_01CyEq51DigmsmwGsa7fEqWS