Skip to content

editorial: [aria.js] generate roleInfo after respec runs#2769

Open
pkra wants to merge 5 commits into
mainfrom
ariajs20260419
Open

editorial: [aria.js] generate roleInfo after respec runs#2769
pkra wants to merge 5 commits into
mainfrom
ariajs20260419

Conversation

@pkra
Copy link
Copy Markdown
Member

@pkra pkra commented Apr 19, 2026

🚀 Netlify Preview:
🔄 this PR updates the following sspecs:

Rewrites buildRoleInfo.js to make it independent of aria.js
by using the "compiled" specification (after respec runs).
Note: adds several hacks to keep bugs alive (to simplify review).

- .github/worflows/roleInfo.yml
  - run on gh-pages pushes (to get latest respec result)
  - checkout main
  - checkout index.html from gh-pages (to get latest respec result)
  - simplify buildRoleInfo.js call
  - add reset of index.html
- common/script/ariaPreprocessing.js
  - duplicate prohibited and deprecated status for global properties
     on list items to preserve it after resolveReferences removes
      *ref elements.
- common/script/buildRoleInfo.js
  - complete rewrite to run independently from aria.js
  - collects states&props
  - loops through role sections and processes the characteristics tables
  - NOTE: several hacks to recreate bugs (to simplify review)
- common/script/roleInfo.js
  - regenerate for review
  - all changes are either
    - removal of duplicates
    - change of position

Part of #2501
@pkra pkra added the editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo label Apr 19, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 19, 2026

Deploy Preview for wai-aria ready!

Name Link
🔨 Latest commit 86912ad
🔍 Latest deploy log https://app.netlify.com/projects/wai-aria/deploys/6a05eeaf6c4a5f0008c56ca2
😎 Deploy Preview https://deploy-preview-2769--wai-aria.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

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

@pkra pkra mentioned this pull request Apr 19, 2026
28 tasks
@pkra pkra marked this pull request as ready for review April 19, 2026 18:31
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 19, 2026

🚀 Deployed on https://deploy-preview-2769--wai-aria.netlify.app

@github-actions github-actions Bot temporarily deployed to pull request April 19, 2026 18:31 Inactive
push:
branches:
- main
- gh-pages
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't know how to test this without merging. Also, this may help with or may be blocked by #2768.

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.

Only way would be through setting up a fork

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I thought this is the cleanest way: when a version is published to github, we update roleinfo.

Comment thread common/script/roleInfo.js
required: false,
disallowed: false,
deprecated: false,
},
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This diff is worse than my local diff was. Reviewers may want to try to paste this version into their local editor for diffing.

There are a few simple de-duplications as well as a few larger groups (gridcell changes are the most "confusing" part).

Remove output hack after rewrite.
@spectranaut
Copy link
Copy Markdown
Contributor

Hoping to take a look at this next week!

node-version: "latest"
- run: npm i linkedom prettier@3.6.0
- run: node ./common/script/buildRoleInfo.js > ./common/script/roleInfo.js
- run: node ./common/script/buildRoleInfo.js
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.

I am not clear how this check would work. In PRs you don't have the respec esported spec, previews are built at run time.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh! You're right. My brain clearly stopped working at some point.

I'll fix it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@daniel-montalvo I've updated this to run respec before buildRoleInfo (and resetting index.html before diffing roleInfo.js)

push:
branches:
- main
- gh-pages
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.

Only way would be through setting up a fork

Comment thread common/script/buildRoleInfo.js
@github-project-automation github-project-automation Bot moved this to Agenda+ in ARIA Editors May 12, 2026
@pkra pkra moved this from Agenda+ to aria.js etc in ARIA Editors May 12, 2026
@pkra pkra moved this from aria.js etc to Agenda+ in ARIA Editors May 12, 2026
Copy link
Copy Markdown
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

Ok, I just read through this all and ran it locally, these changes overall seem fine! Lolcry at the hacks though. Are you thinking of removing "currentlyBroken" as a next step?

I remain confused about the extra logic for marking things are deprecated thought, see comments.

link => {
const name = link.textContent.split(' ')[0]; //TODO: hack because roletype has (state) inside link (as the only role to have that), cf. TODO: in ariaPreprocessing.js
const prop = structuredClone(statesAndProps[name]);
if (key !== 'roletype') prop.deprecated = false; // TODO: should roletype have deprecated=true when the spec lists everything as supported?
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.

Can you explain this comment/situation to me... probably a silly question but I'm having trouble wrapping around why is deprecated always set to false, when there are entries in roleInfo.js where deprecated is true?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If an attribute shows up in a characteristics table, then it's not deprecated on this role.

(For role=roletype, roleInfo lists the 4 attributes "deprecated as global in 1.2" as deprecated: true - hence the TODO).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, I should add: I'm not saying this is correct (looking again now, I think it's not but I haven't had enough coffee yet).

I'm just trying to reproduce roleInfo.js as much as possible right now, with code that's hopefully easier to reason about. (And if we have identified a bug here, then I'll take that as a win 🥳 )

Comment thread common/script/buildRoleInfo.js Outdated
- improve notes on running respec
- remove dead code (identified by review)
@pkra
Copy link
Copy Markdown
Member Author

pkra commented May 13, 2026

@spectranaut

Are you thinking of removing "currentlyBroken" as a next step?

Yes. I hope to discuss it on today's editors call.

@github-actions github-actions Bot temporarily deployed to pull request May 13, 2026 07:41 Inactive
- add respec to npm install
- run respec before buildRoleInfo.js
- checkout index.html before git diff
@github-actions github-actions Bot temporarily deployed to pull request May 13, 2026 08:27 Inactive
@pkra
Copy link
Copy Markdown
Member Author

pkra commented May 14, 2026

Discussed in the last editors' call https://www.w3.org/2026/05/13-aria-editors-minutes.html#e0aa

- buildRoleInfo.js
  - remove hack to reproduce bug in old implementation.
- roleInfo.js: manual update
@pkra
Copy link
Copy Markdown
Member Author

pkra commented May 14, 2026

@spectranaut as discussed I've removed the "main" hack and regenerated roleInfo.js.

Copy link
Copy Markdown
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo

Projects

Status: Agenda+

Development

Successfully merging this pull request may close these issues.

3 participants