Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[selectors] :link and :visited are not mutually exclusive in implementations #2037

Open
victoriasu opened this issue Dec 4, 2017 · 13 comments
Labels
selectors-4 Current Work

Comments

@victoriasu
Copy link

victoriasu commented Dec 4, 2017

According to the spec, :link and :visited should be mutually exclusive, however in:

Chrome, Firefox, and Safari

  • Style rules for :link and :visited are being applied to the same element (a visited link)

Edge

  • All links are using the :link style and not the :visited style

Tested using:
https://jsfiddle.net/victoriaytsu/ujz85sor/

@lilles
Copy link
Member

lilles commented Dec 4, 2017

This is for privacy reasons. See: https://developer.mozilla.org/en-US/docs/Web/CSS/Privacy_and_the_:visited_selector

Your spec link says:

"Since it is possible for style sheet authors to abuse the :link and :visited pseudo-classes to determine which sites a user has visited without the user’s consent, UAs may treat all links as unvisited links or implement other measures to preserve the user’s privacy while rendering visited and unvisited links differently."

So Edge is doing the former thing and Chrome/Firefox/Safari are doing the "other measures"?

@emilio
Copy link
Collaborator

emilio commented Dec 4, 2017

Yeah, querySelector and such never match :visited on Gecko. Similarly getComputedStyle doesn't return :visited styles and such.

@dbaron
Copy link
Member

dbaron commented Dec 4, 2017

For a single selector matching operation, I think they still are mutually exclusive, though. At least based on what I originally described.

@FremyCompany
Copy link
Contributor

I think the difference in Edge is just an invalidation issue for ":link" because we are supposed to behave like other browsers here. https://jsfiddle.net/gck56f1h/ shows we still apply an outline but with wrong color once we visit the link, it doesn't make much sense to me.

@FremyCompany
Copy link
Contributor

Though actually loading the demo in other browsers, I am not sure I understand their behavior either. I think the problem is that you cannot remove all ":link" styles when the link is visited because otherwise this might leak somehow, so only a couple of properties will match using ":visited" and other properties will match using ":link" and this creates a weird state.

Maybe saying ":link" always matches would help solve the craziness but there might be a few websites broken by specificity of ":link" overriding their ":visited" styles.

@FremyCompany
Copy link
Contributor

Looking at the test case a bit more, here is what I understand the difference between Edge and other browsers to be:

Edge:

  • Apply all :link rules and, for allowed properties, also apply all :visited rules (at the same time)

Other browsers:

  • Apply all rules that do not have :visited (including :link rules)
  • Apply all rules that have :visited or do have neither :link nor :visited

The difference is the cascade order. Effectively, a :link rule can override a :visited rule in Edge if its specificity is higher. In Firefox and Chrome, it cannot because this is done as part of a clean second pass.

https://jsfiddle.net/ngtby4te/ shows this (lime outline but yellow background in Edge)

Perf-wise, our behavior seems better. Not sure this is something we want to fix, I am yet to see any issue appear because of that.

@w3c w3c deleted a comment from emilio Dec 6, 2017
@css-meeting-bot
Copy link
Member

css-meeting-bot commented Dec 7, 2017

The Working Group just discussed [css-grid] Choose a single option for resolving padding and maring percent values of grid items

The full IRC log of that discussion <dael> Topic: [css-grid] Choose a single option for resolving padding and maring percent values of grid items
<fantasai> github: https://github.com//issues/2085
<dael> fantasai: Rossen_ are you okay if we publish cr without hte one you just found?
<dael> Rossen_: I really want resolution on this
<dael> fantasai: Can we do it in jan? If we don't get it today it doesn't get published this year. BUt we need to publish, grid is super out of date, it hasn't publsihed since may.
<dael> fantasai: Unless you want to block on this for publication
<dael> Rossen_: I won't object to publishing for this issue.
<dael> fantasai: Let's resolve to publish CR update. Then we can publish again in May.
<dael> Rossen_: Do we not havea call next week?
<dael> fantasai: This is the deadline for CR publication.
<dael> Rossen_: I'm fine dsicussing next week.
<fantasai> Disposition of comments at https://drafts.csswg.org/css-grid-1/issues-cr-2016
<dael> astearns: proposal is publish an updated CR of grid minus this issue. fantasai put together tests for all the issues since we resolved to require them. SO I think we should try to publish.
<dael> astearns: Obj to asking for a new CR of grid?
<dael> RESOLVED: Request publication of new Grid CR
<dael> github: https://github.com//issues/2037
<dael> nainar: We found it weird that edge is doing something different then the rest of us
<dael> nainar: It is technically doing what spec says
<dael> TabAtkins: I don't htink it is.
<nainar> s/nainar/victoria
<nainar> she is not on IRC unfortunately
<nainar> Victoria Su is an intern working under Eric.
<dael> TabAtkins: I think I agree with dbaron it should be exclusive. I thought it depended on which, it's one or the other not a mix.
<dael> TabAtkins: It's not agreeing with how we thought the model worked for this
<dael> Rossen_: I was seeing if fremy is on.
<fantasai> oh, wow that's super interesting testcases
<fremy> don't worry
<fremy> everything is in the issue
<fremy> my mike doesnt seem to be working
<dael> astearns: Given fremy last comment in issue, do we need a spec change to more closely define what other browsers do or is it just clear edge is doing something weird?
<fremy> TabAtkins: not true
<dael> TabAtkins: I think spec is clear that Edge is weird. They're clearly mutually exclusive conditions.
<fremy> TabAtkins: you still need to apply :link { padding } for instance
<dael> astearns: fremy disagrees
<dael> TabAtkins: I think his disagrees that it's an imporant difference
<dael> dbaron: I think fremy thing is after selecto matching. Because the way the property restrictions work for some properties you always get the thing as if it's just visit, but for something like color you use the styles for has visited. That was derrived from the legacy where we didn't have property restrictions
<dael> Rossen_: fremy is coming to my office so you can hear him.
<dael> fremy: I think...tbh i think we do agree edge is different and we need to change. But I don't think the spec clearly defined what should be done. dbaron documents those
<dael> fremy: i think it would be good to clarify the spec so people get it right, esp because there's privacy involved. IT's clear to me you need to match link values for visited links
<dael> TabAtkins: The spec says "these two states are mutually exclusive" so I don't know how ot make it clearer
<dael> fremy: In practice they're not mutually exclusive
<dael> fantasai: What fremy says is try because you're doing background color for visited at same time as color from link. We should think of an easier way to describe this in the spec. For some properties visited doesn't applya nd link applies.
<dael> fremy: They're mutuallye xlcusive for some not all properties. I think that needs to be in the spec
<dael> dbaron: It's a bit dangerious to mix it into the selector. It is a layer aboe selector matching in gecko. If you mix it into selector matching you get different results potentially
<dael> fantasai: I need an example
<TabAtkins> Yeah, impls don't seem to match the spec, I agree now.
<dael> astearns: We're out of time. There will need to be some changes to the spec to clarify and edge will need to change to match the spec
<dael> TabAtkins: No one matches the spec. So something has to be done.
<dael> Rossen_: So we agree to do work.
<dael> astearns: So that's where we need to leave this. I appologize that we didn't get to :focus-ring and :focus-indicator. We'll continue in the issue and try and resolve there.

@tabatkins
Copy link
Member

Wow, we messed up the topics for this. I'll move the comment over and delete this one.

@FremyCompany
Copy link
Contributor

FremyCompany commented Dec 7, 2017

@victoriasu So, I just want to take an opportunity to clarify my thinking.

I do believe that if we keep the spec text as it is today, we should at the very least add a note mentioning that for most properties, :link will apply regardless of visited status, and point to DBaron's spec. I understand @dbaron's point of view that says that, for the purpose of the selector matching, :link and :visited are mutually exclusive (as in you cannot ever have :link:visited match), I just think that for an author reading the spec, it is important to understand that :link will, in practice, match all links except for a few select properties.


To return to the interop issue highlighted in the first post of this thread:

Globally, how it works in Edge is we kinda have a "color" and a "visited-color" property, and when a style rule applies "color:red" we set both of them; if the style came from a :visited rule however, we only set "visited-color" (we do this for a limited subset of properties, for other properties, a visited rule will not set anything when applied; converserly, when a rule contains :link we should only set the non-visited property, though whether we do this for all properties is unclear). This means that :link applies all the times for most properties, and does not apply for properties for which we got the ":link sets only the non-visited field" rule right (when we do not :visited rules are still allowed to override these :link declarations that also happen to have set the visited-property, they just need higher specificity).

It looks like background-color is a property for which we didn't get the ":link does not set the :visited part" right, hence the interop difference in the demo above. For all other properties I tested, we behave like other browsers, so it is just a bug on us to fix this issue.

To conclude, I think as a result of this, only an editorial change should be necessary: adding a note to the spec would be great, if only because the current text caused enough confusion for browser engineers to file this issue in the first place.

@tabatkins
Copy link
Member

Okay, I adapted @dbaron’s spec into actual spec text in PR #2105.

victoriasu pushed a commit to victoriasu/csswg-drafts that referenced this issue Dec 13, 2017
UAs may display chosen colors for :visited and :link elements, but only
return the :link colors from getComputedStyle(). For non-color
properties, UAs may treat all links as unvisited.

fixes w3c#2037
@byung-woo
Copy link
Member

I recently started prototyping ':has()' pseudo class in Chrome, and there was a mailing thread about the interaction of ':has()' and ':visited' in terms of privacy issues.

Although the ':visited' discussion seems to be going on in another thread, and the ':has()' prototyping is very early stage, I thought it might be worth sharing the ':has()' related approach here because it is based on the current chrome ':visited' privacy approach.

The current approach to privacy considerations for ':visited' in the ':has()' argument selector is:

  • The ':visited' selector does not match if it is inside the ':has()' argument selector to prevent leaking visitedness to the link's ancestors by ':has()'. So if a ':has()' argument selector requires a matching ':visited', the style rule will not be applied.

@emilio
Copy link
Collaborator

emilio commented Jun 9, 2021

@byung-woo Why do the existing mitigations not work? On Firefox, we match the link as both visited and unvisited. I don't think :has() introduces any new risk in that regard?

@byung-woo
Copy link
Member

The case at above is the :visited in the :has() argument.

For example, when we have :has(:visited) {...}, the subject elements of the rule are the ancestors of the visited link element. In this case, the rule should not be applied because the ancestor elements are not link. The ':visited' in ':visited .a { ... }' will match, but the ':visited' in '.a:has(:visited)' will not match to prevent leaking visitedness out of the link elements.

It can be said that this approach follows the current chrome approach which only allows the 'descendant combinator' or 'child combinator' for the ':visited' match, because the meaning of ':has()' is technically 'ancestor combinator' or 'preceding sibling combinator'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
selectors-4 Current Work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants