[Webkit-unassigned] [Bug 137418] Implement closest() API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=137418





--- Comment #3 from Benjamin Poulain <benjamin at webkit.org>  2014-10-04 00:26:49 PST ---
(From update of attachment 239265)
View in context: https://bugs.webkit.org/attachment.cgi?id=239265&action=review

The patch is in good shape. There is a bug not mentioned by Darin or I but I would prefer you to discover it through tests. Good test coverage is extremely important to validate a feature.

> LayoutTests/fast/selectors/closest-general.html:43
> +shouldBe('theTarget.closest("#theTarget")', "theTarget");
> +shouldBe('theTarget.closest("ancestor")', "ancestor");
> +shouldBeNull('theTarget.closest("hihi")', "ancestor");

You need a few more test cases to be sure everything works properly.

First, you need to test all the combinators:
    a b
    a+b
    a>b
    a~b
Ideally, you should test a use case that finds an ancestor, and one that does not.

The other kind of use case you need to test is coma separated selectors.
For example:
    a, b
    b, a

Finally, it would be good to have negative and positive cases for the big kind of matching: 
    #id
    .class
    [attribute]
    tagname

> LayoutTests/fast/selectors/closest-general.html:50
> +shouldBeTrue('failOnEmptyArguments()');
> +shouldBeTrue('failOnEmptyStringArgument()');

You should use shouldThrow() to test the exceptions.

There are other cases to cover here, invalid selectors for example ".123", "    ", ")", "()", "}", etc, etc.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.