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

VERIFIED FIXED in mozilla7

Status

()

Core
CSS Parsing and Computation
P1
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

Trunk
mozilla7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

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.
Created attachment 531829 [details] [diff] [review]
part 1.  Move the class decl for nsHTMLLabelElement out to a header.
Attachment #531829 - Flags: review?(Olli.Pettay)
Created attachment 531830 [details] [diff] [review]
part 2.  Make labels expose a nicer API for getting their labeled content.
Attachment #531830 - Flags: review?(Olli.Pettay)
Created attachment 531831 [details] [diff] [review]
part 3.  Set :hover and :active state for labeled elements when their label has that state.
Attachment #531831 - Flags: review?(dbaron)
Whiteboard: [need review]

Updated

6 years ago
Attachment #531829 - Flags: review?(Olli.Pettay) → review+

Comment 4

6 years ago
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.
Attachment #531830 - Flags: review?(Olli.Pettay) → review+
> And could you keep element strong here.

Done.
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
Attachment #531831 - Flags: review?(dbaron) → review+
Whiteboard: [need review] → [need landing]
http://hg.mozilla.org/projects/cedar/rev/b6b39d52b33e
http://hg.mozilla.org/projects/cedar/rev/2e27b606caed
http://hg.mozilla.org/projects/cedar/rev/9ab424c86a26
Flags: in-testsuite+
Whiteboard: [need landing] → fixed-in-cedar
Target Milestone: --- → mozilla7
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
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-cedar
Hello.

Could anyone provide, if the case is, some simple STR or a test case to verify the issue?
data:text/html,<style>:hover { color: green; }</style><label for="x">Hover me</label><input id="x" value="I should turn green">
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.
Status: RESOLVED → VERIFIED
Depends on: 710917
You need to log in before you can comment on or make changes to this bug.