Closed Bug 1377469 Opened 7 years ago Closed 7 years ago

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

Categories

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

53 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: bzbarsky, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
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 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 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+
Attachment #8888156 - Attachment is obsolete: true
(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.
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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee: nobody → emilio
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: