1377469 - stylo: Links should not inherit from their parent's visited style context

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P2)

Description

[Boris Zbarsky [:bzbarsky]]

Gecko's GetContext has:

  if (aFlags & eIsLink) {
    // If this node is a link, we want its visited's style context's
    // parent to be the regular style context of its parent, because
    // only the visitedness of the relevant link should influence style.
    parentIfVisited = aParentContext;
  }

Stylo doesn't have anything equivalent at the moment.

This is causing the inherit-keyword-1.xhtml subtest of test_visited_reftests.html to fail.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Looks like bug 1381635 will re-enable this test.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Never mind, looks like bug 1381635 is unrelated to this.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Yeah, this should be relatively easy to fix... Let me just do that.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1377469: Don't inherit from the parent visited style if we're a link.

Review commit: https://reviewboard.mozilla.org/r/159070/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/159070/

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1377469: Update reftest expectations.

Review commit: https://reviewboard.mozilla.org/r/159210/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/159210/

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8888156 [details]
Bug 1377469: Don't inherit from the parent visited style if we're a link.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159070/diff/1-2/

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8888257 [details]
Bug 1377469: Update reftest expectations.

https://reviewboard.mozilla.org/r/159210/#review164660

::: layout/style/test/stylo-failures.md:68
(Diff revision 1)
>  * test_css_supports.html: issues around @supports syntax servo/servo#15482 [2]
>  * test_author_specified_style.html: support serializing color as author specified bug 1348165 [27]
>  * browser_newtab_share_rule_processors.js: agent style sheet sharing [1]
>  * :visited support bug 1381635
> -  * test_visited_reftests.html `inherit-keyword-1.xhtml` [2]
>    * ... `mathml-links.html` [2]

Please update this line to replace `...` with `test_visited_reftests.html`.  (You could also mention bug 1371030 since that's filed for it.)

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8888156 [details]
Bug 1377469: Don't inherit from the parent visited style if we're a link.

https://reviewboard.mozilla.org/r/159070/#review164666

Thanks for working on this! :)

::: servo/components/style/style_resolver.rs:477
(Diff revision 2)
>  
>          if self.element.skip_root_and_item_based_display_fixup() {
>              cascade_flags.insert(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP);
>          }
>          if cascade_visited.visited_dependent_only() {
> +            if pseudo.is_some() || !self.element.is_link() {

A comment to explain this condition would be great to have!  Gecko's `nsStyleSet::GetContext` attempts to explain their version of similar logic, and I think it's helpful for future readers.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8888257 [details]
Bug 1377469: Update reftest expectations.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159210/diff/1-2/

Comment 11

[Emilio Cobos Álvarez (:emilio)]

https://github.com/servo/servo/pull/17801

Comment 12

[Emilio Cobos Álvarez (:emilio)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #9)
> A comment to explain this condition would be great to have!  Gecko's
> `nsStyleSet::GetContext` attempts to explain their version of similar logic,
> and I think it's helpful for future readers.

Err, whoops, the PR is in the queue already, but I'll land the comment.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7707a729e9c8
Update reftest expectations. r=jryans

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7707a729e9c8