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

Inconsistent :hover background rendering (manual testcase) #16559

Closed
sciguyryan opened this issue Apr 21, 2017 · 8 comments
Closed

Inconsistent :hover background rendering (manual testcase) #16559

sciguyryan opened this issue Apr 21, 2017 · 8 comments

Comments

@sciguyryan
Copy link

@sciguyryan sciguyryan commented Apr 21, 2017

Hi guys.

Using the manual test included below. Hover over the first list element and it will correctly render with a red background. We would expect the same to be true of the second but this isn't the case with Servo. It is intermittently rendered with a red background as the mouse is moved around within the visual bounds of the element but it isn't consistently kept red while the cursor is within the visual bounds of the element. If either the overflow or position rules are removed it renders correctly.

This case renders as expected in both Firefox and Chrome.

<!DOCTYPE html>
<html>
<head>
    <title>testcase</title>
        <style>
        li {
            overflow: hidden;
            border-bottom: 1px solid #000;
            position: relative;
        }
        li:hover {
            background: #f00;
        }
        </style>
</head>
<body>
    <ul>
        <li>test</li>
        <li>test 2</li>
    </ul>
</body>
</html>

Unfortunately it isn't really possible to screenshot this effect as it only happens when hovering but I was able to provide a gif snapshot of the screen while performing the test above.

hover behaviour

This was found on the latest nightly build.

@sciguyryan
Copy link
Author

@sciguyryan sciguyryan commented Apr 24, 2017

This problem still seems to exist in the most recent Servo nightly builds.

@jdm
Copy link
Member

@jdm jdm commented Apr 29, 2017

This regressed at some point between 533853f and ef5c61e.

@jdm
Copy link
Member

@jdm jdm commented Apr 29, 2017

This is a regression from #16336. cc @mrobinson

@mrobinson
Copy link
Member

@mrobinson mrobinson commented May 1, 2017

I think the issue here is that hit testing is no longer taking into account overflow:hidden clipping, since the previous code wasn't taking into account overflow:scroll clipping before. This should be possible to fix by testing whether the cursor position is clipped out when processing scroll roots.

@sciguyryan
Copy link
Author

@sciguyryan sciguyryan commented May 31, 2017

This appears to have been fixed in the latest nightlies. I'm not sure when exactly but it is no longer an issue!

Yipee!

@sciguyryan
Copy link
Author

@sciguyryan sciguyryan commented May 31, 2017

Does this need a new test added to cover potential regressions?

1 similar comment
@sciguyryan
Copy link
Author

@sciguyryan sciguyryan commented May 31, 2017

Does this need a new test added to cover potential regressions?

@jdm
Copy link
Member

@jdm jdm commented May 31, 2017

I don't think we can test hover behavior yet.

@jdm jdm closed this May 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.