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

Incorrect assertions in HTMLInputElement tests? #17792

Open
droeh opened this issue Jul 11, 2019 · 4 comments · May be fixed by #20330
Open

Incorrect assertions in HTMLInputElement tests? #17792

droeh opened this issue Jul 11, 2019 · 4 comments · May be fixed by #20330

Comments

@droeh
Copy link

droeh commented Jul 11, 2019

I'm adding support for the capture attribute to Gecko and I'm a bit confused by two assertions:

assert_attribute_log_entry(logEntries.last(), {name: 'capture', oldValue: '', newValue: 'user', namespace: null});

assert_attribute_log_entry(logEntries.last(), {name: 'capture', oldValue: '', newValue: 'asdf', namespace: null});

Shouldn't the oldValue in both of these assertions be null rather than an empty string? It doesn't seem to me that capture is getting set anywhere prior in order for the old value to be non-null.

@gsnedders
Copy link
Member

cc/ @Honry who wrote these tests, admittedly years ago. (also @domenic and @annevk in case they know)

@annevk
Copy link
Member

annevk commented Nov 19, 2019

The other problem with this test is the if ('capture' in HTMLInputElement.prototype) { conditional. If this is a tentative test it should be marked as such, but it shouldn't contain optional parts.

@domenic
Copy link
Member

domenic commented Nov 19, 2019

Yes, I think it should be null instead of empty string. And also the file should be renamed to something like HTMLInputElement-capture, and the conditional-ness removed.

Honry added a commit to Honry/web-platform-tests that referenced this issue Nov 20, 2019
@Honry Honry linked a pull request Nov 20, 2019 that will close this issue
@Honry
Copy link
Contributor

Honry commented Nov 20, 2019

I just drop the PR #20330, PTAL, thanks!

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

Successfully merging a pull request may close this issue.

5 participants