Add :visited rules to DOMUtils getCSSStyleRules()

ASSIGNED
Assigned to

Status

()

P3
normal
ASSIGNED
7 years ago
4 months ago

People

(Reporter: harth, Assigned: heycam)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [devtools-platform])

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
From bug 713106, we need a way to get all the CSS rules that apply to an element, including :visited styles.
(Reporter)

Updated

7 years ago
Blocks: 713106
There was just another report for this in the Firebug issue tracker.[1] Is there any ETA?

Sebastian

[1] http://code.google.com/p/fbug/issues/detail?id=7283
(Reporter)

Comment 2

5 years ago
(In reply to Sebastian Zartner from comment #1)
> There was just another report for this in the Firebug issue tracker.[1] Is
> there any ETA?

Since noone is working on it, unknown.
Component: DOM → DOM: CSS Object Model
(Assignee)

Updated

4 years ago
Assignee: nobody → cam
Status: NEW → ASSIGNED
Comment on attachment 8451327 [details] [diff] [review]
patch

It's not clear to me that this is the most useful API to expose to the caller, but it is easy to expose, so r=dbaron.

Could you add comments to the IDL file explaining that getCSSStyleRules and getVisitedDependentCSSStyleRules simply return different lists, one of which is the list of rules if unvisited, and the other of which is the list of rules if visited?

I suspect it would be useful if getVisitedDependentCSSStyleRules also had an out boolean that said whether the list differed from getCSSStyleRules (i.e., whether the rule nodes are different).  It might also be useful to have an additional boolean that says whether there is a style-if-visited (which isn't quite the same as asking whether an ancestor has differences, in the case of nested links).

I guess I'm also not crazy about the name for getVisitedDependentCSSStyleRules.  Maybe drop the "Dependent"?  I'm not sure if that's any better, though.
Attachment #8451327 - Flags: review?(dbaron) → review+
(Assignee)

Comment 5

4 years ago
(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #4)
> It's not clear to me that this is the most useful API to expose to the
> caller, but it is easy to expose, so r=dbaron.

Are you thinking that a function that returns the union of the style-if-unvisited and style-if-visited rules would be more helpful?

> I suspect it would be useful if getVisitedDependentCSSStyleRules also had an
> out boolean that said whether the list differed from getCSSStyleRules (i.e.,
> whether the rule nodes are different).  It might also be useful to have an
> additional boolean that says whether there is a style-if-visited (which
> isn't quite the same as asking whether an ancestor has differences, in the
> case of nested links).

What would the latter be useful for?

> I guess I'm also not crazy about the name for
> getVisitedDependentCSSStyleRules.  Maybe drop the "Dependent"?  I'm not sure
> if that's any better, though.

getIfVisitedCSSStyleRules or getCSSStyleRulesIfVisited?  (I copied the "VisitedDependent" naming from nsStyleContext::GetVisitedDependentColor.)
(Assignee)

Comment 6

4 years ago
(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #4)
> Could you add comments to the IDL file explaining that getCSSStyleRules and
> getVisitedDependentCSSStyleRules simply return different lists, one of which
> is the list of rules if unvisited, and the other of which is the list of
> rules if visited?

Is it possible for StyleIfVisited to return a different style context, but with the same rule node as the original style context?
Flags: needinfo?(dbaron)
Yes.  The simplest case of that would be for the child of a link; there will be two style contexts with the same rule node, but their parents will have different rule nodes.
Flags: needinfo?(dbaron)
(Assignee)

Comment 8

4 years ago
(In reply to Cameron McCormack (:heycam) from comment #5)
> Are you thinking that a function that returns the union of the
> style-if-unvisited and style-if-visited rules would be more helpful?

I think I should do this actually, since that's really what bug 713106 wants.
(Assignee)

Comment 9

4 years ago
David, if we want a method that can return the rules that apply regardless of the visited state of any element, that we should add a new nsStyleSet function that does something like:

  * set up a TreeMatchContext that somehow indicates that :link and :visited should always match
  * do most of what nsStyleSet::ResolveStyleFor does without actually returning an nsStyleContext
  * return the rule node we end up at after walking all the rules

?
(Assignee)

Updated

4 years ago
Flags: needinfo?(dbaron)
(In reply to Cameron McCormack (:heycam) from comment #5)
> (In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from
> comment #4)
> > It's not clear to me that this is the most useful API to expose to the
> > caller, but it is easy to expose, so r=dbaron.
> 
> Are you thinking that a function that returns the union of the
> style-if-unvisited and style-if-visited rules would be more helpful?

Probably, yes.

> > I suspect it would be useful if getVisitedDependentCSSStyleRules also had an
> > out boolean that said whether the list differed from getCSSStyleRules (i.e.,
> > whether the rule nodes are different).  It might also be useful to have an
> > additional boolean that says whether there is a style-if-visited (which
> > isn't quite the same as asking whether an ancestor has differences, in the
> > case of nested links).
> 
> What would the latter be useful for?

Figuring out whether there might be inherited styles that would differ based on :visited state.

> > I guess I'm also not crazy about the name for
> > getVisitedDependentCSSStyleRules.  Maybe drop the "Dependent"?  I'm not sure
> > if that's any better, though.
> 
> getIfVisitedCSSStyleRules or getCSSStyleRulesIfVisited?  (I copied the
> "VisitedDependent" naming from nsStyleContext::GetVisitedDependentColor.)

Maybe.

(GetVisitedDependentColor is the function that actually applies the knowledge of whether the relevant link is visited or not, and thus returns information that depends on visitedness, which is privacy-sensitive information.)

(In reply to Cameron McCormack (:heycam) from comment #9)
> David, if we want a method that can return the rules that apply regardless
> of the visited state of any element, that we should add a new nsStyleSet
> function that does something like:
> 
>   * set up a TreeMatchContext that somehow indicates that :link and :visited
> should always match

Yes; we already have this:  nsRuleWalker::eLinksVisitedOrUnvisited.

>   * do most of what nsStyleSet::ResolveStyleFor does without actually
> returning an nsStyleContext

Also without doubling the work to handle getting a second rule node for the if-visited style.  So it might look rather more like nsStyleSet::HasStateDependentStyle except with a call to FileRules instead of WalkRuleProcessors.

>   * return the rule node we end up at after walking all the rules

Yep.
Flags: needinfo?(dbaron)
(In reply to Cameron McCormack (:heycam) from comment #8)
> (In reply to Cameron McCormack (:heycam) from comment #5)
> > Are you thinking that a function that returns the union of the
> > style-if-unvisited and style-if-visited rules would be more helpful?
> 
> I think I should do this actually, since that's really what bug 713106 wants.

Can you explain exactly what you mean by this comment?  What would be great is if getCSSStyleRules was smart enough to include the visited rules if and only if they are currently applied, which I think may be what you mean.
Flags: needinfo?(cam)
(Assignee)

Comment 12

4 years ago
I believe I meant, rather than the current patch on this bug, which adds a new function that returns the list of rules that apply under the assumption that the target element's closest <a> ancestor is visited, that we instead add a new function that returns the list of rules that could apply regardless of visited state of all ancestor link elements, because I thought that is actually what bug 713106 wants to show in the style inspector.

Can you confirm whether that or a function that only includes those visited rules that are currently matching (as you say in comment 11) would be more useful?
Flags: needinfo?(cam) → needinfo?(bgrinstead)
(In reply to Cameron McCormack (:heycam) from comment #12)
> I believe I meant, rather than the current patch on this bug, which adds a
> new function that returns the list of rules that apply under the assumption
> that the target element's closest <a> ancestor is visited, that we instead
> add a new function that returns the list of rules that could apply
> regardless of visited state of all ancestor link elements, because I thought
> that is actually what bug 713106 wants to show in the style inspector.
> 
> Can you confirm whether that or a function that only includes those visited
> rules that are currently matching (as you say in comment 11) would be more
> useful?

I was assuming we'd want a UI that only shows it if it's applied, but it's been a while and I may be forgetting some context.  Patrick, do you think :visited styles always show up in the rule view regardless of if it's been visited?
Flags: needinfo?(bgrinstead) → needinfo?(pbrosset)
(In reply to Brian Grinstead [:bgrins] from comment #13)
> I was assuming we'd want a UI that only shows it if it's applied, but it's
> been a while and I may be forgetting some context.
Sorry for the delay responding.
You're right Brian, my understanding of bug 713106 is that we need a way to show the :visited styles only when they actually apply to the currently selected element.
Flags: needinfo?(pbrosset)
One tricky part about this may be knowing when to refresh the view to show new rules if a link becomes :visited by opening it in a new tab while it's already selected in the rule view.  Not sure if there is a notification for that.

Maybe it's possible by listening for all new windows being created and comparing them against the URL of the currently selected link, and then just doing a full rule view refresh if it does match.  I'm guessing that wouldn't catch all cases but if we could cover the most common case it'd probably be good enough
(In reply to Cameron McCormack (:heycam) from comment #12)
> Can you confirm whether that or a function that only includes those visited
> rules that are currently matching (as you say in comment 11) would be more
> useful?

Yes, as mentioned in Comment 13 and 14.  Everyone I've talked to agrees that we should only be showing these rules when they are applied.  How difficult do you think that will be?
Flags: needinfo?(cam)
Whiteboard: [devtools-platform]
See Also: → bug 1211613
See Also: → bug 1384099
See Also: bug 1384099
(Assignee)

Comment 17

6 months ago
I'm sorry for dropping this.  Patrick, can you tell me whether devtools still needs an API like this?  Would someone in your or Panos' team want to pick this up?
Flags: needinfo?(cam) → needinfo?(pbrosset)
Thanks Cameron for starting the work on this. I'll put it on my radar and get someone on the team to take a look.
Flags: needinfo?(pbrosset)

Updated

4 months ago
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.