[webkit-reviews] review denied: [Bug 7555] :hover style not applied on hover if its display property is different from original style's : [Attachment 6828] Patch

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sat Mar 4 13:11:14 PST 2006

Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 7555: :hover style not applied on hover if its display property is
different from original style's

Attachment 6828: Patch

------- Additional Comments from Darin Adler <darin at apple.com>
Since attach() could do arbitrary amounts of work, I think this code could
malfunction if there's a new hover node or active node. For example, a modal
dialog might even run inside the attach().

But I have an idea about how to fix this in a way that's cleaner and more

We should factor the code to set the hover and active nodes out of
RenderLayer:: updateHoverActiveState. That function could simply decide what
the new nodes should be given the NodeInfo and then call a function in
DocumentImpl to actually change the state of all the bits on the nodes.

After all, the booleans are effectively caches of the hoverNode and activeNode
setting on the document so they should be maintained together.

The code in ElementImpl could then call that same function to restore the hover
and active node before calling attach(). The only strange thing about this is
that it would set the hover and active node to things that don't yet have a

After calling attach() it can then call a function that will move the node off
of anything that doesn't have a renderer. That function, too, can be in

That way, all this code to maintain hover and active nodes can be together
inside DocumentImpl, and the flags won't ever be out of sync. with the nodes in
the document.

I think this approach will let you do the bug fix you're doing here and keep
the code a little easier to read.

What do you think?

More information about the webkit-reviews mailing list