Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Add test of cursor on element underneath a <select> menu #800

Merged
merged 1 commit into from Sep 3, 2015
Merged

Add test of cursor on element underneath a <select> menu #800

merged 1 commit into from Sep 3, 2015

Conversation

@syncbot
Copy link
Collaborator

syncbot commented Jul 11, 2015

Automatic validation checks of commit eca00da passed.

@syncbot
Copy link
Collaborator

syncbot commented Jul 11, 2015

Automatic validation checks of commit feaec41 passed.

@cvrebert
Copy link
Member Author

CC: @frivoal, since you're an Editor of css-ui-3

@cvrebert
Copy link
Member Author

CC: @tantek for review, as other Editor of CSS3 UI

@frivoal
Copy link

frivoal commented Jul 26, 2015

@cvrebert Sorry for the sluggish answer, I was on vacation.

  1. Why are use using testharness.js, since you're not making any programmatic assertions?

  2. No need to ask for the test to be skipped if cursor is not supported. This goes into the test suite for css-ui, so cursor is supposed to be supported, and failing the test if it is not is appropriate.

  3. The test passes if cursor is not supported at all. You should display a recognizable cursor shape or the FAIL case, rather than rely on just the default.

  4. I completely agree with the intent of the test, but I am not sure I can find justification for it in specifications, on 2 grounds:
    4.1: The spec defines being over the element in terms of the "border edge". I can't find a clearcut answer to whether that's supposed to include the thing that pops down when you click on the select element.
    4.2: More glaringly, the spec does not seem to define what happens when elements overlap.

Can you find justifications somewhere for these two points? If not, I still agree with the test, but then we probably need to add extra clarifications in the specification (this level or the next) first to back it up.

@syncbot
Copy link
Collaborator

syncbot commented Jul 27, 2015

Automatic validation checks of commit 164ea24 discovered the following problem:

In css-ui-3/select-cursor-001-manual.html:

  • Not linked to a tracked specification anchor.

@syncbot
Copy link
Collaborator

syncbot commented Jul 27, 2015

Automatic validation checks of commit 6d5a56d passed.

@cvrebert
Copy link
Member Author

  1. Why are use using testharness.js, since you're not making any programmatic assertions?

Sorry, http://testthewebforward.org/docs/test-templates.html#manual-test confused me. Removed the <script>s.

  1. No need to ask for the test to be skipped if cursor is not supported.

Removed.

@cvrebert
Copy link
Member Author

@frivoal

  1. The test passes if cursor is not supported at all. You should display a recognizable cursor shape or the FAIL case, rather than rely on just the default.

Given that the spec says:

User agents may ignore the cursor property over [...] native UI widgets e.g. those that may be used inside some user agent specific implementations of form elements.

Then setting e.g. select { cursor: XXX; } (like you're suggesting) cannot be relied upon.
Also, my sanity-check testcase is merely checking that insanity (using the cursor of an underlapping non-ancestor element) doesn't occur. I think it'd be reasonable to leave stronger "verify that cursor is actually supported" checks to other testcases.

4.2: More glaringly, the spec does not seem to define what happens when elements overlap.

I agree, that appears to be a significant oversight in the spec. Perhaps it could delegate and define the semantics in terms of :hover (or define them both in terms of some 3rd shared definition)? I'm unable to come up with a situation where foo { cursor: XXX; } and foo:hover { cursor: XXX; } wouldn't be equivalent.

If not, I still agree with the test, but then we probably need to add extra clarifications in the specification (this level or the next) first to back it up.

Cool. Do you need me file a spec bug or anything?

@syncbot
Copy link
Collaborator

syncbot commented Jul 27, 2015

Automatic validation checks of commit b2a1ffe passed.

@cvrebert
Copy link
Member Author

cvrebert commented Aug 9, 2015

Went ahead and posted to the mailing list. As of yet, there haven't been any comments on the post.

@cvrebert
Copy link
Member Author

@frivoal Friendly ping. Hope your move went okay.

@frivoal
Copy link

frivoal commented Aug 16, 2015

@cvrebert Filing a bug on the spec is a good idea, and the best way to do that is what you did: sending a mail to www-style. Your mail is in my queue of things to get to, but due to my ongoing move, I'm processing this queue rather slowly. Sorry for the delay, and thanks again for the test and the bug report.

@cvrebert
Copy link
Member Author

cvrebert commented Sep 2, 2015

@frivoal Given that you've now updated the spec to use hit testing and thus resolve (4), and I responded to (3) in #800 (comment) , is there anything else blocking you from merging this?

frivoal added a commit that referenced this pull request Sep 3, 2015
Add test of `cursor` on element underneath a <select> menu
@frivoal frivoal merged commit 52f09e4 into w3c:master Sep 3, 2015
@cvrebert cvrebert deleted the select-cursor-ie branch September 3, 2015 01:48
@frivoal
Copy link

frivoal commented Sep 3, 2015

@cvrebert Nope, nothing is blocking. The delay was only caused by me trying to get connectivity in my new home. Since that's solved, this is merged. Thanks.

@cvrebert
Copy link
Member Author

cvrebert commented Sep 3, 2015

Thank you.

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

Successfully merging this pull request may close these issues.

None yet

3 participants