Fix tests for existing elements #311

Merged
merged 1 commit into from Nov 27, 2013

Conversation

Projects
None yet
2 participants
@lucianafujii
Contributor

lucianafujii commented Nov 20, 2013

The type of a just created HTMLElement is never undefined, it is
actually an object of type HTMLUnknownElement. So now instead of
comparing to undefined, the tests check if the element is an instance of
HTMLUnknownElement and fails if it is.

Fix tests for existing elements
The type of a just created HTMLElement is never undefined, it is
actually an object of type HTMLUnknownElement. So now instead of
comparing to undefined, the tests check if the element is an instance of
HTMLUnknownElement and fails if it is.
@NielsLeenheer

This comment has been minimized.

Show comment
Hide comment
@NielsLeenheer

NielsLeenheer Nov 20, 2013

You are right that there should be an extra check for instanceof HTMLUnknownElement. On browsers that follow HTML5 tree creation algorithm unknown elements are and instance of HTMLUnknownElement which is a descendant of HTMLElement. Just checking for instanceof HTMLElement will always pass.

On older browsers this does work because before the adoption of the HTML5 unknown elements were not an descendant of HTMLElement.

The check for typeof HTMLElement != 'undefined' was there originally for a reason. Not all browsers have a generic HTMLElement defined. Of course modern browsers do, but if you run the test on I think S60 or S40 Webkit instanceof HTMLElement gives an error. That error is now also handled by the try {} catch() {}, so it could indeed be removed.

You are right that there should be an extra check for instanceof HTMLUnknownElement. On browsers that follow HTML5 tree creation algorithm unknown elements are and instance of HTMLUnknownElement which is a descendant of HTMLElement. Just checking for instanceof HTMLElement will always pass.

On older browsers this does work because before the adoption of the HTML5 unknown elements were not an descendant of HTMLElement.

The check for typeof HTMLElement != 'undefined' was there originally for a reason. Not all browsers have a generic HTMLElement defined. Of course modern browsers do, but if you run the test on I think S60 or S40 Webkit instanceof HTMLElement gives an error. That error is now also handled by the try {} catch() {}, so it could indeed be removed.

This comment has been minimized.

Show comment
Hide comment
@lucianafujii

lucianafujii Nov 20, 2013

Owner
Owner

lucianafujii replied Nov 20, 2013

This comment has been minimized.

Show comment
Hide comment
@NielsLeenheer

NielsLeenheer Nov 20, 2013

I think the patch is good as-is. But before landing I want to check if it works as expected on older devices.

I think the patch is good as-is. But before landing I want to check if it works as expected on older devices.

NielsLeenheer added a commit that referenced this pull request Nov 27, 2013

Merge pull request #311 from lucianafujii/version-5.0
Fix tests for existing elements

@NielsLeenheer NielsLeenheer merged commit dffbfd8 into WebPlatformTest:version-5.0 Nov 27, 2013

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