656379 - CSS Hover and active should propagate from labels to the labeled content

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

We already do this for native theming; see bug 426082.

Doing this makes it easier to fix bug 598833.

The patches I attach will have a bug that's actually a bit of a pain to fix before bug 598833 lands: they won't handle dynamic removal/insertion of controls labeled by a currently hovered or active label well.  I'll fix that in bug 656377.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Move the class decl for nsHTMLLabelElement out to a header.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Make labels expose a nicer API for getting their labeled content.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Set :hover and :active state for labeled elements when their label has that state.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 531830 [details] [diff] [review]
part 2.  Make labels expose a nicer API for getting their labeled content.


> void
> nsHTMLLabelElement::PerformAccesskey(PRBool aKeyCausesActivation,
>                                      PRBool aIsTrustedEvent)
> {
>   if (!aKeyCausesActivation) {
>-    nsCOMPtr<nsIContent> content = GetControlContent();
>-    if (content)
>-      content->PerformAccesskey(aKeyCausesActivation, aIsTrustedEvent);
>+    Element* element = GetLabeledElement();
>+    if (element)
>+      element->PerformAccesskey(aKeyCausesActivation, aIsTrustedEvent);
{
}

And could you keep element strong here. So nsRefPtr<Element> element.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> And could you keep element strong here.

Done.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 531831 [details] [diff] [review]
part 3.  Set :hover and :active state for labeled elements when their label has that state.

r=dbaron

Comment 7

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/projects/cedar/rev/b6b39d52b33e
http://hg.mozilla.org/projects/cedar/rev/2e27b606caed
http://hg.mozilla.org/projects/cedar/rev/9ab424c86a26

Comment 8

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/b6b39d52b33e
http://hg.mozilla.org/mozilla-central/rev/2e27b606caed
http://hg.mozilla.org/mozilla-central/rev/9ab424c86a26

Comment 9

[Virgil Dicu [:virgil] [QA]]

Hello.

Could anyone provide, if the case is, some simple STR or a test case to verify the issue?

Comment 10

[Boris Zbarsky [:bzbarsky]]

data:text/html,<style>:hover { color: green; }</style><label for="x">Hover me</label><input id="x" value="I should turn green">

Comment 11

[Virgil Dicu [:virgil] [QA]]

Thank you for the feedback.

Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0

Verified fixed on Ubuntu 11.04, Windows XP, Windows 7 and Mac OS 10.6 with test case provided in comment 10.