Bug 1488155 - Only no-op visited <-> unvisited changes. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 05 Sep 2018 12:52:16 +0000
changeset 483111 3430ddb28e8408ec10e625494dcc21264d70a8ac
parent 483110 618c88011cd5410ba779ef6f223b40c4d3aca7a9
child 483112 1616abd4c12f3d5ca793c0d332b1738e93803ddd
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersheycam
bugs1488155
milestone64.0a1
Bug 1488155 - Only no-op visited <-> unvisited changes. r=heycam Other changes should really be (and are) indistinguishable. Differential Revision: https://phabricator.services.mozilla.com/D4847
layout/base/RestyleManager.cpp
layout/reftests/bugs/1488155-ref.html
layout/reftests/bugs/1488155.html
layout/reftests/bugs/reftest.list
servo/components/style/invalidation/element/state_and_attributes.rs
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -3179,17 +3179,19 @@ RestyleManager::ContentStateChanged(nsIC
 
   Element& element = *aContent->AsElement();
   if (!element.HasServoData()) {
     return;
   }
 
   const EventStates kVisitedAndUnvisited =
     NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED;
-  if (aChangedBits.HasAtLeastOneOfStates(kVisitedAndUnvisited) &&
+  // NOTE: We want to return ASAP for visitedness changes, but we don't want to
+  // mess up the situation where the element became a link or stopped being one.
+  if (aChangedBits.HasAllStates(kVisitedAndUnvisited) &&
       !Gecko_VisitedStylesEnabled(element.OwnerDoc())) {
     aChangedBits &= ~kVisitedAndUnvisited;
     if (aChangedBits.IsEmpty()) {
       return;
     }
   }
 
   nsChangeHint changeHint;
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1488155-ref.html
@@ -0,0 +1,7 @@
+<!doctype html>
+<style>
+  :link {
+    color: green;
+  }
+</style>
+<a href>Should be green</a>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1488155.html
@@ -0,0 +1,14 @@
+<!doctype html>
+<html class="reftest-wait">
+<style>
+  :link {
+    color: green;
+  }
+</style>
+<a id="target">Should be green</a>
+<script>
+window.addEventListener("MozReftestInvalidate", function() {
+  target.setAttribute("href", "");
+  document.documentElement.className = "";
+});
+</script>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -2077,8 +2077,9 @@ pref(layout.css.moz-document.url-prefix-
 test-pref(layout.css.prefixes.gradients,false) == 1451874.html 1451874-ref.html
 == 1456111-1.html about:blank
 test-pref(layout.css.contain.enabled,false) == 1466008.html 1466008-ref.html
 fuzzy(0-1,0-625) == 1466638-1.html 1466638-1-ref.html
 == bug1472465-1.html bug1472465-1-ref.html
 == 1475971-1.html 1475971-1-ref.html
 == 1483649-1.xul 1483649-1-ref.xul
 test-pref(layout.css.contain.enabled,true) == 1483946.html 1483946-ref.html
+test-pref(layout.css.visited_links_enabled,false) == 1488155.html 1488155-ref.html
--- a/servo/components/style/invalidation/element/state_and_attributes.rs
+++ b/servo/components/style/invalidation/element/state_and_attributes.rs
@@ -150,32 +150,21 @@ where
 
         let state_changes = wrapper.state_changes();
         let snapshot = wrapper.snapshot().expect("has_snapshot lied");
 
         if !snapshot.has_attrs() && state_changes.is_empty() {
             return false;
         }
 
-        // If we are sensitive to visitedness and the visited state changed, we
-        // force a restyle here. Matching doesn't depend on the actual visited
-        // state at all, so we can't look at matching results to decide what to
-        // do for this case.
-        if state_changes.intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE) &&
-            self.shared_context.visited_styles_enabled
-        {
+        // If we the visited state changed, we force a restyle here. Matching
+        // doesn't depend on the actual visited state at all, so we can't look
+        // at matching results to decide what to do for this case.
+        if state_changes.intersects(ElementState::IN_VISITED_OR_UNVISITED_STATE) {
             trace!(" > visitedness change, force subtree restyle");
-            // We shouldn't get here with visited links disabled, but it's hard
-            // to assert in cases where you record a visitedness change and
-            // afterwards you change some of the stuff (like the pref) that
-            // changes whether visited styles are enabled.
-            //
-            // So just avoid the restyle here, because it kind of would kill the
-            // point of disabling visited links.
-            //
             // We can't just return here because there may also be attribute
             // changes as well that imply additional hints for siblings.
             self.data.hint.insert(RestyleHint::restyle_subtree());
         }
 
         let mut classes_removed = SmallVec::<[Atom; 8]>::new();
         let mut classes_added = SmallVec::<[Atom; 8]>::new();
         if snapshot.class_changed() {