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

RESOLVED FIXED in Firefox 56

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
24 days ago
3 days ago

People

(Reporter: bz, Unassigned)

Tracking

(Blocks: 2 bugs)

53 Branch
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

24 days ago
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.
Duplicate of this bug: 1371034
Blocks: 1324348
(Reporter)

Updated

24 days ago
Priority: -- → P2
Looks like bug 1381635 will re-enable this test.
Depends on: 1381635
Never mind, looks like bug 1381635 is unrelated to this.
No longer depends on: 1381635
Yeah, this should be relatively easy to fix... Let me just do that.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 8

4 days ago
mozreview-review
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.)
Attachment #8888257 - Flags: review?(jryans) → review+

Comment 9

4 days ago
mozreview-review
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.
Attachment #8888156 - Flags: review?(jryans) → review+
Comment hidden (mozreview-request)
Attachment #8888156 - Attachment is obsolete: true
https://github.com/servo/servo/pull/17801
(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

4 days ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7707a729e9c8
Update reftest expectations. r=jryans
https://hg.mozilla.org/mozilla-central/rev/7707a729e9c8
Status: NEW → RESOLVED
Last Resolved: 3 days ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.