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

DOMTokenList: separate whitespace trim/duplicate removal; add versions #6691

Conversation

queengooborg
Copy link
Contributor

In DOMTokenList, we have an entry about how modifications will trim the whitespace and remove duplicates, with partial implementation notes for IE and Edge about how they trim whitespace but not remove duplicates. However, from my testing, I've found that Chrome and Firefox also implemented whitespace trimming before duplicate removal. This PR separates the subfeature into two subfeatures, and adds/fixes versions for both of them.

@github-actions github-actions bot added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Sep 15, 2020
@foolip
Copy link
Contributor

foolip commented Sep 17, 2020

This will remove an entry, so adding the needs-release-note label.

@foolip
Copy link
Contributor

foolip commented Sep 17, 2020

@vinyldarkscratch can you link to the tests you used for this?

@queengooborg
Copy link
Contributor Author

queengooborg commented Sep 17, 2020

Certainly! I used the following pieces of JavaScript to test manually in BrowserStack:

trim_whitespace:

var elm = document.createElement('b');
elm.className = ' foo bar foo ';
elm.classList.remove('bar');
console.log(elm.className === 'foo foo' || elm.className === 'foo');

remove_duplicates:

var elm = document.createElement('b');
elm.className = ' foo bar foo ';
elm.classList.remove('bar');
console.log(return elm.className === 'foo');

@foolip
Copy link
Contributor

foolip commented Sep 19, 2020

@vinyldarkscratch the remove_duplicates seems to assume that if duplicates are removes then whitespace is also trimmed. That's probably true, but did you check what the elm.className was in the latest build where it wasn't 'foo'? A value like ' foo foo ' is not impossible to imagine in some implementation where these things were done in the other order.

@queengooborg
Copy link
Contributor Author

I checked all the browser versions, and have confirmed that white space trimming came first or at the same time in all the browsers, and that duplicate removal had come later.

@ddbeck ddbeck requested a review from foolip September 24, 2020 15:21
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

I'll leave the review of the data to Philip, but I had a couple nitpicky style suggestions. Thank you!

queengooborg and others added 2 commits September 29, 2020 20:23
Co-authored-by: Daniel D. Beck <daniel@ddbeck.com>
Co-authored-by: Daniel D. Beck <daniel@ddbeck.com>
@queengooborg queengooborg merged commit 111760d into mdn:master Oct 2, 2020
@queengooborg queengooborg deleted the api/DOMTokenList/whitespace-and-duplicates branch October 2, 2020 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants