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

Removing attribute sprouting #181

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from
Open

Removing attribute sprouting #181

wants to merge 1 commit into from

Conversation

cyns
Copy link

@cyns cyns commented Jul 20, 2021

per discussion in 6.29.2021 AOM meeting, removing attribute sprouting

per discussion in 6.29.2021 AOM meeting, removing attribute sprouting
@cookiecrook
Copy link
Collaborator

What happens with the following?

el.setAttribute('aria-labelledby', 'original_label_idref');
el.ariaLabelledByElements = [newLabelEl]; // I don't recall if this is the current syntax for setting, but my point remains.
el.getAttribute('aria-labelledby'); // Is this stale content attribute left in the DOM, removed, or set to an empty string?

I think what should happen is that setting the ariaLabelledByElements DOM property should NOT leave the stale aria-labelledby content attribute in the DOM... Instead, it should either remove the attribute, or set it to an empty string.

@rniwa
Copy link

rniwa commented Aug 17, 2021

What happens with the following?

el.setAttribute('aria-labelledby', 'original_label_idref');
el.ariaLabelledByElements = [newLabelEl]; // I don't recall if this is the current syntax for setting, but my point remains.
el.getAttribute('aria-labelledby'); // Is this stale content attribute left in the DOM, removed, or set to an empty string?

I think what should happen is that setting the ariaLabelledByElements DOM property should NOT leave the stale aria-labelledby content attribute in the DOM... Instead, it should either remove the attribute, or set it to an empty string.

I might be wrong but I think we landed on not affecting content attributes in this case as well to be consistent with not sprouting content attributes. It's weird for DOM API to not create a content attribute but at the same time to remove an existing one.

@cyns
Copy link
Author

cyns commented Aug 17, 2021

TODO: Add example of attribute removal

Copy link

@rniwa rniwa left a comment

Choose a reason for hiding this comment

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

r=me

@rniwa
Copy link

rniwa commented Aug 17, 2021

Let's make a change to mention removal of relationship in a separate PR.

@alice
Copy link
Member

alice commented Oct 3, 2021

To clarify my original intent with sprouting: I imagined that where a valid IDREF/IDREF list couldn't be generated, an empty content attribute would still be sprouted, and that removing the content attribute would have the effect of removing the association. That seems to have been dropped sometime before this change.

My reasoning for that was that the presence/absence of the content attribute would reliably indicate that an IDL attribute was/wasn't set.

Separately, sprouting serializable attributes using IDREFs where possible was more of a nice to have, and I'm happy for that to be removed if it's causing serious issues.

I had a brief skim through the meeting notes for the last 5 months, but couldn't find the discussion of the first point (empty content attributes) easily. Would anyone like to point me to that?

(This is not a -1 for this PR, just a clarification of the original intent of this design.)

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