Skip to content

Conversation

jonathanKingston
Copy link
Contributor

@jonathanKingston jonathanKingston commented Oct 13, 2025

Asana Task/Github Issue: https://app.asana.com/1/137249556945/project/72649045549333/task/1211632579258684?focus=true

Description

  • Trim links when blank (and setting is enabled).
  • Adds support for images but disables then also.
  • Makes ol behave like ul.
  • Adds a fallback when main is empty.
  • Ignores iframes that the page doesn't own and avoids exceptions being triggered for sandboxed frames (even if we caught them they end up in the console).

Testing Steps

Checklist

Please tick all that apply:

  • I have tested this change locally
  • I have tested this change locally in all supported browsers
  • This change will be visible to users
  • I have added automated tests that cover this change
  • I have ensured the change is gated by config
  • This change was covered by a ship review
  • This change was covered by a tech design
  • Any dependent config has been merged

Note

Enhances DOM-to-Markdown (images, link trimming, ol/ul parity), hardens iframe access, adds body fallback, and introduces jsdom-based tests with fixtures.

  • Page Context / DOM-to-Markdown:
    • Export checkNodeIsVisible and domToMarkdown; add trimBlankLinks and allow excludeSelectors to be null.
    • Support img to markdown and treat ol like ul; improve li formatting via collapseAndTrim.
    • Update link rendering to trim blank links when enabled and preserve spacing; add getAttributeOrBlank helper.
  • Iframe Handling:
    • Pre-check sandbox and origin in getSameOriginIframeDocument to avoid security errors; only include same-origin content when allowed.
  • Content Extraction:
    • Add img to excluded inert elements by default; introduce body fallback (bodyFallback) when main content is too short/empty.
    • Refactor extraction via closure; pass trimBlankLinks and includeIframes flags; improved logging.
  • Tests:
    • Add page-context-dom.spec.js using jsdom with fixtures and expected markdown outputs.
    • Ignore generated test outputs in .gitignore.
  • Tooling:
    • Add jsdom as a dev dependency; update lockfile.

Written by Cursor Bugbot for commit 047130f. This will update automatically on new commits. Configure here.

Copy link

netlify bot commented Oct 13, 2025

Deploy Preview for content-scope-scripts ready!

Name Link
🔨 Latest commit 047130f
🔍 Latest deploy log https://app.netlify.com/projects/content-scope-scripts/deploys/68f04e84b4b89200085e730f
😎 Deploy Preview https://deploy-preview-2011--content-scope-scripts.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

github-actions bot commented Oct 13, 2025

Temporary Branch Update

The temporary branch has been updated with the latest changes. Below are the details:

Please use the above install command to update to the latest version.

Copy link

github-actions bot commented Oct 13, 2025

[Beta] Generated file diff

Time updated: Thu, 16 Oct 2025 01:47:38 GMT

Apple
    - apple/contentScope.js

File has changed

Integration
    - integration/contentScope.js

File has changed

Windows
    - windows/contentScope.js

File has changed

@jonathanKingston jonathanKingston marked this pull request as ready for review October 14, 2025 09:45
@jonathanKingston jonathanKingston requested review from a team and noisysocks as code owners October 14, 2025 09:45
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

noisysocks
noisysocks previously approved these changes Oct 15, 2025
Copy link
Contributor

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Code looks good.

I tested this by npm linking into macOS and browsing to nba.com. I didn’t see [](/) in the chat history like I did when testing without this branch.

Now that this code is getting closer to landing in production we should think about tests. Snapshot tests (e.g. toMatchInlineSnapshot()) at the domToMarkdown level would be a great cost vs. benefit imo.

@jonathanKingston
Copy link
Contributor Author

Now that this code is getting closer to landing in production we should think about tests. Snapshot tests (e.g. toMatchInlineSnapshot()) at the domToMarkdown level would be a great cost vs. benefit imo.

Yeah agreed, I'll be doing that once back I already scoped: https://app.asana.com/1/137249556945/project/72649045549333/task/1211654081298449?focus=true If I get time before I will address.

noisysocks
noisysocks previously approved these changes Oct 16, 2025
cursor[bot]

This comment was marked as outdated.

@jonathanKingston jonathanKingston added this pull request to the merge queue Oct 16, 2025
@jonathanKingston jonathanKingston removed this pull request from the merge queue due to a manual request Oct 16, 2025
@jonathanKingston jonathanKingston force-pushed the jkt/trimLinksAndIncludeImages branch from d8e91ea to ce3c984 Compare October 16, 2025 00:59
Copy link
Contributor

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

🥳

@jonathanKingston jonathanKingston added this pull request to the merge queue Oct 16, 2025
Merged via the queue into main with commit 1f66b13 Oct 16, 2025
21 checks passed
@jonathanKingston jonathanKingston deleted the jkt/trimLinksAndIncludeImages branch October 16, 2025 02:23
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.

2 participants