729310 - Add :visited rules to DOMUtils getCSSStyleRules()

Status: RESOLVED DUPLICATE of bug 713106

(Core :: DOM: CSS Object Model, defect, P3)

Description

[Heather Arthur [:harth]]

From bug 713106, we need a way to get all the CSS rules that apply to an element, including :visited styles.

Comment 1

[Sebastian Zartner [:sebo]]

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

Comment 2

[Heather Arthur [:harth]]

(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.

Comment 3

[Cameron McCormack (:heycam)]

Attachment: patch

https://tbpl.mozilla.org/?tree=Try&rev=2323be0924fb

Comment 4

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

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.

Comment 5

[Cameron McCormack (:heycam)]

(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.)

Comment 6

[Cameron McCormack (:heycam)]

(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?

Comment 7

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

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.

Comment 8

[Cameron McCormack (:heycam)]

(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.

Comment 9

[Cameron McCormack (:heycam)]

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

?

Comment 10

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

(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.

Comment 11

[Brian Grinstead [:bgrins]]

(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.

Comment 12

[Cameron McCormack (:heycam)]

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?

Comment 13

[Brian Grinstead [:bgrins]]

(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?

Comment 14

[Patrick Brosset <:pbro>]

(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.

Comment 15

[Brian Grinstead [:bgrins]]

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

Comment 16

[Brian Grinstead [:bgrins]]

(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?

Comment 17

[Cameron McCormack (:heycam)]

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?

Comment 18

[Patrick Brosset <:pbro>]

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.

Comment 19

[Tim Nguyen :ntim]

Duplicate of bug 713106 ?

Comment 20

[Daisuke Akatsuka (:daisuke)]

Hello!
According to the comments in this bug, because the purpose of this bug is for bug 713106, it seems that we can close as duplicate.