Open Bug 1120399 Opened 10 years ago Updated 4 months ago

Let pseudo-class locking API handle :hover and :active ancestor applying

Categories

(Core :: DOM: Core & HTML, defect, P5)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

People

(Reporter: sebo, Unassigned)

References

Details

(Whiteboard: domcore-bugbash-triaged, )

The :hover and :active pseudo-classes not only apply to the hovered/active element but also to all it's ancestors.[1] Hover the 'child' <div> in this example data:text/html,<!DOCTYPE html><style type="text/css">%23parent, %23child {width:100px;height:100px;text-align:center} %23parent {background-color:%236464ff;} %23child {position:absolute;top:50px;left:150px;background-color:%23c8c8ff;} %23parent:hover {background-color:yellow;} %23child:hover{background-color:yellow;}</style><div id="parent">parent<div id="child">child</div></div> and you'll see that both, the child and the parent, get yellow. The inIDOMUtils functions addPseudoClassLock() and removePseudoClassLock() in contrast only work on the current element. To see that open Scratchpad on the page above and execute the following code in the browser environment: let DOMUtils = Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils); let node = content.document.getElementById("child"); DOMUtils.addPseudoClassLock(node, ":hover"); So for consistency the APIs should behave the same as the browser Sebastian [1] http://dev.w3.org/csswg/selectors-4/#the-hover-pseudo
This is really a matter of use cases. Is the idea to expose a low-level primitive that lets you lock a particular pseudo-class on a particular element and build other things on top of it, or is the idea to expose some sort of higher-level API? Right now we have the former. Once we start doing the latter, we end up with some questions we need to answer about the desired behavior of the high-level API. For example, how should locking :hover on an element interact with actual mouse movement? Right now there is no interaction; the lock stays no matter what you do with the mouse. But if we lock the whole ancestor chain there's a good chance that this will break the optimizations around notifying on hover state changes for ancestors. Also, what is the desired interaction with DOM mutations? If the element with locked :hover is moved in the DOM, should the hover state stay locked on the old ancestor chain or move to the new ancestor chain? Similar for inputs when their label is in locked hover state but then the label changes which input it's labeling and whatnot, though I expect this to be a much rarer case. It's not obvious to me that all consumers of this API for :hover want identical answers to these questions.
> Is the idea to expose a low-level primitive that lets you lock a particular pseudo-class on a > particular element and build other things on top of it, or is the idea to expose some sort of > higher-level API? The point of this bug is to change the behavior of the existing APIs to reflect the GUI behavior in Firefox and the CSS Selectors spec. Is there any usage for this API to only apply a pseudo-class to one specific element? Note that the DevTools currently lock the ancestor chain manually calling the functions on every ancestor element. Sebastian
> The point of this bug is to change the behavior Yes, I understand that. Note that you didn't answer any of my questions.
(In reply to Boris Zbarsky [:bz] from comment #1) > Is the idea to expose a low-level > primitive that lets you lock a particular pseudo-class on a particular > element and build other things on top of it, or is the idea to expose some > sort of higher-level API? Again, the idea is to change the behavior of the existing APIs to work like the spec in regard of ancestor elements. If the current functionality should still be available, then the new behavior should be implemented by adding an optional parameter to them. > For example, how should locking :hover on an element interact with actual mouse movement? > Right now there is no interaction; the lock stays no matter what you do with > the mouse. There also won't be an interaction on ancestors then. > Also, what is the desired interaction with DOM mutations? If the element > with locked :hover is moved in the DOM, should the hover state stay locked > on the old ancestor chain or move to the new ancestor chain? I see two solutions for this: 1. The locked state will be moved to the new ancestor chain. 2. The locked state will be removed. > Similar for inputs when their label is in locked hover state but then the label changes > which input it's labeling When the label changes which input it's labeling, the locked state should also be updated. > It's not obvious to me that all consumers of this API for :hover want > identical answers to these questions. As far as I can see[1][2] the consumers of this API are all development related, namely the built-in devtools, Firebug, DOM Inspector and Firefox OS 1.2 Simulator. While the devtools cover the ancestor highlighting, they do not handle DOM manipulations at the moment. So in case this doesn't get implemented in the APIs, it would need to be fixed in them. Firebug and DOM Inspector currently don't cover ancestor highlighting nor DOM manipulations. At least for Firebug I can say that the behavior described above is what it would expect, but for DOM Inspector it's probably the same as the use case is the same. I don't know the use case of Firefox OS 1.2 Simulator. Sebastian [1] https://mxr.mozilla.org/mozilla-central/search?string=addPseudoClassLock [2] https://mxr.mozilla.org/addons/search?string=addPseudoClassLock
> 1. The locked state will be moved to the new ancestor chain. How does this work if there is an explicit, as opposed to implicit, lock on one of the ancestors already? As in, I lock an element into hover, then I lock one of its descendants. Then I move the descendant to no longer be under this ancestor. For that matter, what if I lock an ancestor, then a descendant, then unlock the descendant. Should the ancestor now be unlocked? > When the label changes which input it's labeling, the locked state should also be > updated. What if the input had been locked independently? This is what I meant by it not being obvious that all consumers want the same answers to this stuff, by the way.
(In reply to Boris Zbarsky [:bz] from comment #5) > > 1. The locked state will be moved to the new ancestor chain. > > How does this work if there is an explicit, as opposed to implicit, lock on > one of the ancestors already? > > As in, I lock an element into hover, then I lock one of its descendants. > Then I move the descendant to no longer be under this ancestor. > > For that matter, what if I lock an ancestor, then a descendant, then unlock > the descendant. Should the ancestor now be unlocked? I'd say any explicit locks on ancestors will be removed once a descendant gets an explicit lock set. Use case is that people actually wanted to set the lock on the descendant and not on the ancestor. > > When the label changes which input it's labeling, the locked state should also be > > updated. > > What if the input had been locked independently? According to the solution above the explicit lock on the input will be removed as soon as the explicit lock is set on the label. Sebastian
Again, is that what EVERY SINGLE consumer of this API wants?
(In reply to Boris Zbarsky [:bz] from comment #7) > Again, is that what EVERY SINGLE consumer of this API wants? Asking people behind DevTools, DOM Inspector and Firefox OS Simulator for feedback. Sebastian
Flags: needinfo?(philip.chee)
Flags: needinfo?(past)
Flags: needinfo?(fayearthur)
I'm not the right person for this, but I know who is.
Flags: needinfo?(pbrosset)
Flags: needinfo?(past)
Flags: needinfo?(mratcliffe)
Flags: needinfo?(fayearthur)
Flags: needinfo?(bgrinstead)
(In reply to Panos Astithas [:past] from comment #9) > I'm not the right person for this, but I know who is. Similarly for the DOM Inspector a better person to NEEDINFO is Neil Rashbrook
Flags: needinfo?(philip.chee)
DOM Inspector used to call setContentState (which I think used to handle :hover and :active ancestors) but as it hasn't worked for focus since Gecko 1.9.2 I switched it to using the pseudo-class lock API for Gecko 12 and later. Nobody seems to have complained that it no longer locks ancestors (I've only wanted to look at the content state of one element at the time) but obviously we wouldn't have any problem if the behaviour was changed back, so to speak.
There are 2 things in devtools that use addPseudoClassLock/removePseudoClassLock: - The SimpleOutlineHighlighter which adds a lock on a *single* element (the pseudo-class being a special devtools one: :-moz-devtools-highlighted) -> This is used when our usual highlighters can't be used (in XUL windows for instance). It just draws a simple outline around whatever element is highlighted. - The other thing is users can right-click an element in the inspector and choose :hover, :active or :focus, and in that case, the inspector locks the element *and* all of its ancestors. So, we have both use cases really, and changing this API would break our SimpleOutlineHighlighter. Note that when bug 1102276 is fixed, we will have no further use of the SimpleOutlineHighlighter though. So, once done, I don't see any problem with changing this API's behavior. Somewhat related: bug 985517.
Flags: needinfo?(pbrosset)
I'm assuming that the proposal in this bug only applies to :hover/:active. Note that ":focus" does NOT work like those; just because :focus applies to an element doesn't mean it applies to its ancestors (and it doesn't look like devtools apply it up the parent chain). The real question is how you want the interaction to look with the user locking :hover on multiple elements at once, not how you want it to behave for the simple case of locking it on one element.
(In reply to Boris Zbarsky [:bz] from comment #13) > I'm assuming that the proposal in this bug only applies to :hover/:active. Right, the summary already indicates that. (In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #12) > - The SimpleOutlineHighlighter which adds a lock on a *single* element (the > pseudo-class being a special devtools one: :-moz-devtools-highlighted) As pointed out above the suggested change would not affect :-moz-devtools-highlighted. Sebastian
See Also: → 985517
Blocks: 985517
See Also: 985517 →
No longer blocks: 985517
See Also: → 985517
> The real question is how you want the interaction to look with the user > locking :hover on multiple elements at once, not how you want it to behave > for the simple case of locking it on one element. Regarding the behavior in the DevTools inspector, I'd add that we do already have the ability with the existing API to lock multiple elements, but we don't do it because of UX concerns. Right now, we clear all locks when the selected node changes out of concern about the user forgetting that they locked a node. Bug 985517 is filed to change this behavior, which is basically depending on UX work in Bug 1120406.
Flags: needinfo?(bgrinstead)
(In reply to Boris Zbarsky [:bz] from comment #5) > > 1. The locked state will be moved to the new ancestor chain. > > How does this work if there is an explicit, as opposed to implicit, lock on > one of the ancestors already? > > As in, I lock an element into hover, then I lock one of its descendants. > Then I move the descendant to no longer be under this ancestor. > > For that matter, what if I lock an ancestor, then a descendant, then unlock > the descendant. Should the ancestor now be unlocked? > > > When the label changes which input it's labeling, the locked state should also be > > updated. > > What if the input had been locked independently? > > This is what I meant by it not being obvious that all consumers want the > same answers to this stuff, by the way. This does get messy, indeed. Right now we handle all of this stuff a really ad-hoc way that doesn't handle any of these cases at all. For instance, if you have this structure: <body> <div> <h1 /> <div> <body> Then you lock on h1, we loop through and make it so for the ancestors: <body:hover> <div:hover> <h1:hover /> <div> <body> But, if you move the body.appendChild(h1), we end up with a DOM like this (notice that the div is still hovered, which is surely not what anyone wants): <body:hover> <h1:hover /> <div:hover /> <body> Thinking about this more, I wonder if this (and all the use cases bz mentions above) could be handled by keeping track of which elements have explicit locks, then on any mutation just clear all locks and reapply locks to any explicitly-locked elements that are still in the DOM.
(In reply to Brian Grinstead [:bgrins] from comment #16) > Thinking about this more, I wonder if this (and all the use cases bz > mentions above) could be handled by keeping track of which elements have > explicit locks, then on any mutation just clear all locks and reapply locks > to any explicitly-locked elements that are still in the DOM. I completely agree that our tools should clear locks when an element is mutated. In my opinion the current API behavior is correct... I see no reason that setting a pseudoclass should do exactly the same as interacting with the browser. Adding special cases for some pseudoclasses to the API would be sure to result in errors as the collection or pseudoclasses increases.
Flags: needinfo?(mratcliffe)
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #17) > In my opinion the current API behavior is correct... I see no reason that > setting a pseudoclass should do exactly the same as interacting with the > browser. The reason is to allow debugging pseudo-class CSS. If locking a pseudo-class behaves differently to the normal interaction, it causes confusion. It also requires additional clicks in case you want to see :hover and :active pseudo-class information of ancestor elements. > Adding special cases for some pseudoclasses to the API would be > sure to result in errors as the collection or pseudoclasses increases. The API could share the pseudo-class logic of the normal interaction with the browser to avoid such errors. Sebastian
> The API could share the pseudo-class logic of the normal interaction with the browser Not easily, fwiw. We'd basically be reimplementing it twice inside the browser if we do it, because it needs to do different things in normal operation (setting state) and the lock case (setting the lock state)... It's possible we can refactor things somewhat to share code, but it might well be more complex that way too.
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046 Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5. If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML
Severity: minor → S4

Hi Honza,
My team was triaging old bugs in our backlog and we were wondering if this is still needed for Devtools , or if we can close this one. Thank you.

Flags: needinfo?(odvarko)
Whiteboard: domcore-bugbash-triaged
See Also: → 1911377

The related DevTools code still has to handle this by itself, so it looks like InspectorUtils.addPseudoClassLock and InspectorUtils.removePseudoClassLock still work the same.

Also, the bug outlined by Brian in comment 16 can still be reproduced. So I created bug 1911377 for that now, which can either be fixed by turning the pseudo-class lock APIs into high-level APIs (this bug) or by clearing and re-applying the pseudo-class locks as suggested by Brian.

Sebastian

Thank you Sebastian for filing the bugs (bug 1911377 has been triaged).
So, yes, it would be great if this is fixed, but the prirority isn't high.

Flags: needinfo?(odvarko)
Whiteboard: domcore-bugbash-triaged → domcore-bugbash-triaged,
You need to log in before you can comment on or make changes to this bug.