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

[css-variables] [selectors] Should custom properties work for :visited? #2263

Open
emilio opened this issue Feb 2, 2018 · 13 comments
Open
Labels
selectors-4 Current Work

Comments

@emilio
Copy link
Collaborator

emilio commented Feb 2, 2018

Should the following test-case be brown or yellow?

<!doctype html>
<style>
  a {
    --foo: brown;
  }

  :visited {
    --foo: yellow;
    color: var(--foo);
  }
</style>
<a href="">Which color?</a>

Right now Gecko is yellow, blink / webkit are brown.

@upsuper upsuper added the selectors-4 Current Work label Feb 2, 2018
@emilio
Copy link
Collaborator Author

emilio commented Feb 2, 2018

Gecko's behavior seems to be the only one that makes any sense to me, reading the spec.

@SelenIT
Copy link
Collaborator

SelenIT commented Feb 2, 2018

In my opinion, Blink/WebKit's behavior seems to make more sense. Custom properties are not allowed visited properties, so var() uses the cascaded value from other rules matching the link. Given that the purpose of visited link styling limitation is user's privacy protection, this sounds reasonable to me.

In Edge 16, the link is also brown.

@Loirooriol
Copy link
Contributor

CSS Selectors says "record the values of the allowed :visited properties", but it does not specify if the recorded value is the specified one or the computed one. It seems Firefox does the latter and the others do the former.

@emilio
Copy link
Collaborator Author

emilio commented Feb 2, 2018

In my opinion, Blink/WebKit's behavior seems to make more sense. Custom properties are not allowed visited properties, so var() uses the cascaded value from other rules matching the link. Given that the purpose of visited link styling limitation is user's privacy protection, this sounds reasonable to me.

I guess that's fair, yeah... It's somewhat weird that they do variable substitution but not computation.

@SelenIT
Copy link
Collaborator

SelenIT commented Feb 2, 2018

it does not specify if the recorded value is the specified one or the computed one.

IMHO, it shouldn't matter in this case. The --foo: yellow declaration in the :visited rule should be ignored altogether because --foo is not listed among "allowed :visited properties". So either way, the --foo property value for a visited link should be one specified or cascaded to the :link state. Please correct me if I miss something.

@Loirooriol
Copy link
Contributor

should be ignored altogether because --foo is not listed among "allowed :visited properties"

True, but color is listed. So the value of color when the element matches :visited should be recorded. This will be var(--foo) if it's the specified value, or rgb(255, 255, 0) if it's the computed one.

The former will compute to rgb(165, 42, 42) when not matching :visited, the latter will stay as rgb(255, 255, 0).

@emilio
Copy link
Collaborator Author

emilio commented Feb 3, 2018

I think from the spec wording, Firefox is right. "Compute the element's style as if the relevant link was :visited" should include matching, cascading, and resolving variable references. I'm not sure how could you justify that something like:

<!doctype html>
<style>
  :link {
    --foo: brown;
  }

  :visited {
    --foo: yellow;
    color: var(--foo);
  }
</style>
<a href="">Which color?</a>

Should display brown if the link is visited, since clearly computing the style as if the element was :visited would've never matched :link.

@fantasai
Copy link
Collaborator

fantasai commented Feb 3, 2018

@Loirooriol Oof, that's a stray commit. The exact spec text for :visited management is under discussion in #2105 and #2037 but isn't ready yet.

Under the current spec, both interpretations are valid, although ignoring --foo: yellow in the color declaration is more aggressive than necessary. It must, however, not return yellow in the OM, and probably can't return yellow in any property that's not otherwise supported for :visited links, either, as this can leak information.

@Nadya678
Copy link

Nadya678 commented Feb 3, 2018

In my opinion the :visited and :link should be removed from specification because of privacy reason.

@emilio
Copy link
Collaborator Author

emilio commented Feb 4, 2018

I think that's not realistic. When writing Gecko's new style engine we almost broke visited in some subtle ways and people noticed: https://bugzilla.mozilla.org/show_bug.cgi?id=1417781

It'd be nice to do something like https://bugzilla.mozilla.org/show_bug.cgi?id=1398414, ensure it's web-compatible, ship it, and then :visited stops being a privacy issue at all and all this complexity goes away, but not sure that's going to be done any time soon.

@Nadya678
Copy link

Nadya678 commented Feb 4, 2018

Any privacy setting to disable/enable it?

BTW. I think the spec should be created with security in mind.

Same-origin policy? To mark as visited only internal links? Then all styles can work. Visiting of the same origin sub-pages can be detected with other methods. For example onclick or cookies/IP.

@tabatkins
Copy link
Member

CSS Selectors says "record the values of the allowed :visited properties", but it does not specify if the recorded value is the specified one or the computed one.

The commit (that I accidentally pushed earlier; it's still visible from the #2105 PR) specifies in step 2 that you compute styles, then in step 3 you record the styles; the implication is that you're recording computed styles. I can make that more clear.

(The custom property itself is not recorded, and so any properties other than the ":visited properties" won't see the values of custom properties set in a :visited rule. But the :visited properties will, since their var()s get resolved before they're recorded.)

@tabatkins
Copy link
Member

Thus, in @emilio's example, the link is yellow when visited, but a background-image: linear-gradient(var(--foo), white); declaration would create a brown->white gradient (since background-image isn't a :visited property).

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 17, 2023
…shin

This matches the behavior of other browsers (in fact, I filed [1] about
it long time ago).

This avoids a bunch of overhead in some speedometer subtests. Makes me a
bit sad because I still think our approach is slightly more correct per
spec, but not worth the performance cost.

[1]: w3c/csswg-drafts#2263

Differential Revision: https://phabricator.services.mozilla.com/D190705
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Oct 17, 2023
…shin

This matches the behavior of other browsers (in fact, I filed [1] about
it long time ago).

This avoids a bunch of overhead in some speedometer subtests. Makes me a
bit sad because I still think our approach is slightly more correct per
spec, but not worth the performance cost.

[1]: w3c/csswg-drafts#2263

Differential Revision: https://phabricator.services.mozilla.com/D190705
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 17, 2023
…shin

This matches the behavior of other browsers (in fact, I filed [1] about
it long time ago).

This avoids a bunch of overhead in some speedometer subtests. Makes me a
bit sad because I still think our approach is slightly more correct per
spec, but not worth the performance cost.

[1]: w3c/csswg-drafts#2263

Differential Revision: https://phabricator.services.mozilla.com/D190705

UltraBlame original commit: e136cc9893278df1df3e3b997b9e6756122b9c44
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 17, 2023
…shin

This matches the behavior of other browsers (in fact, I filed [1] about
it long time ago).

This avoids a bunch of overhead in some speedometer subtests. Makes me a
bit sad because I still think our approach is slightly more correct per
spec, but not worth the performance cost.

[1]: w3c/csswg-drafts#2263

Differential Revision: https://phabricator.services.mozilla.com/D190705

UltraBlame original commit: e136cc9893278df1df3e3b997b9e6756122b9c44
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 17, 2023
…shin

This matches the behavior of other browsers (in fact, I filed [1] about
it long time ago).

This avoids a bunch of overhead in some speedometer subtests. Makes me a
bit sad because I still think our approach is slightly more correct per
spec, but not worth the performance cost.

[1]: w3c/csswg-drafts#2263

Differential Revision: https://phabricator.services.mozilla.com/D190705

UltraBlame original commit: e136cc9893278df1df3e3b997b9e6756122b9c44
github-actions bot pushed a commit to Loirooriol/stylo that referenced this issue Mar 17, 2024
…shin

This matches the behavior of other browsers (in fact, I filed [1] about
it long time ago).

This avoids a bunch of overhead in some speedometer subtests. Makes me a
bit sad because I still think our approach is slightly more correct per
spec, but not worth the performance cost.

[1]: w3c/csswg-drafts#2263

Differential Revision: https://phabricator.services.mozilla.com/D190705
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

No branches or pull requests

7 participants