Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add class and id mappings #2308

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add class and id mappings #2308

wants to merge 3 commits into from

Conversation

smockle
Copy link
Contributor

@smockle smockle commented Aug 8, 2024

Closes w3c/html-aam#553

Initially filed as w3c/html-aam#559

@aleventhal provided mappings in w3c/html-aam#553. I verified they match up with Chromium’s mappings:

id

class

Reviewers: Please also check the formatting. I’ve attempted to match existing styles, but those are inconsistent, e.g. “Object attributes” is bold in datetime (but not in draggable); it uses <code> in src (but not in abbr).

Previews

Test, Documentation and Implementation tracking Once this PR has been reviewed and has consensus from the working group, tests should be written and issues should be opened on browsers. Add N/A and check when not applicable.
  • "author MUST" tests:
  • "user agent MUST" tests:
  • Browser implementations (link to issue or commit):
  • Does this need AT implementations?
  • Related APG Issue/PR:
  • MDN Issue/PR:

Copy link

netlify bot commented Aug 8, 2024

Deploy Preview for wai-aria ready!

Name Link
🔨 Latest commit d9b8df6
🔍 Latest deploy log https://app.netlify.com/sites/wai-aria/deploys/66c340099959cd0008bd700a
😎 Deploy Preview https://deploy-preview-2308--wai-aria.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 site configuration.

@smockle smockle changed the title Add class and id mappings [html-aam] Add class and id mappings Aug 8, 2024
@rahimabdi rahimabdi removed their request for review August 12, 2024 00:21
@rahimabdi
Copy link
Contributor

Removing myself as reviewer as I just wanted to review the code references.

Copy link
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.

The change looks good, but we should verify that all implementations match it -- not just chrome. @rahimabdi, I added you back as a review because in the WG meeting we asked you to review for webkit, if you don't mind :)

I'll look at firefox now.

html-aam/index.html Outdated Show resolved Hide resolved
@spectranaut
Copy link
Contributor

Ok, I checked Firefox, and Firefox doesn't implement AXDOMClassList on, but otherwise is good.
I think webkit implements both so this is good to land after three positive reviews :)

html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
change `<code>` to just ` marks per (most) other instances
@scottaohara
Copy link
Member

approving pending @rahimabdi's review to just verify the webkit mappings.

once good on that, we can merge.

@spectranaut spectranaut changed the title [html-aam] Add class and id mappings Add class and id mappings Aug 19, 2024
@rahimabdi
Copy link
Contributor

rahimabdi commented Sep 19, 2024

Copy link
Contributor

@rahimabdi rahimabdi left a comment

Choose a reason for hiding this comment

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

WebKit mappings are correct 👍🏾.

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

Successfully merging this pull request may close these issues.

Mappings for class and id attributes missing
4 participants