Do not drop !important declarations from mixed blocks

.changeset/grumpy-olives-float.md

@@ -0,0 +1,7 @@
+---
+"@siteimprove/alfa-cascade": patch
+---
+
+**Fixed:** `!important` declarations in rules with normal declarations are not dropped anymore.
+
+While moving handling of `!important` from `alfa-style` to `alfa-cascade`, a regression was introduced that effectively dropped `!important` declarations from rules having both normal and important ones (e.g. `div {color: blue; background: red !important }`); this is now fixed.

.changeset/wise-maps-end.md

@@ -0,0 +1,5 @@
+---
+"@siteimprove/alfa-cascade": patch
+---
+
+**Added:** An `isImportant` predicate is available on `Origin` and `Precedence`.

packages/alfa-cascade/src/precedence/origin.ts

@@ -57,5 +57,11 @@ export enum Origin {
 export namespace Origin {
   export type JSON = Origin;
 
+  export function isImportant(origin: Origin): boolean {
+    return (
+      Origin.ImportantAuthor <= origin && origin <= Origin.ImportantUserAgent
+    );
+  }
+
   export const compare: Comparer<Origin> = Comparable.compareNumber;
 }

packages/alfa-cascade/src/precedence/precedence.ts

@@ -64,6 +64,12 @@ export namespace Precedence {
     };
   }
 
+  export function isImportant<LAYERED extends boolean>(
+    precedence: Precedence<LAYERED>,
+  ): boolean {
+    return Origin.isImportant(precedence.origin);
+  }
+
   export function toTuple<LAYERED extends boolean>(
     precedence: Precedence<LAYERED>,
   ): [Origin, Encapsulation, boolean, Layer<LAYERED>, Specificity, Order] {

packages/alfa-cascade/src/rule-tree.ts

@@ -6,6 +6,7 @@ import { None, Option } from "@siteimprove/alfa-option";
 import * as json from "@siteimprove/alfa-json";
 
 import { Block } from "./block";
+import { Precedence } from "./precedence";
 
 /**
  * The rule tree is a data structure used for storing the rules that match each
@@ -196,7 +197,9 @@ export namespace RuleTree {
      */
     public add(block: Block): Node {
       // If we have already encountered the exact same selector (physical identity),
-      // we're done.
+      // with declarations of the same importance, we're done.
+      // Because blocks are split by importance, the same selector can appear
+      // once per importance when the rules are grouped.
       // This occurs when the exact same style rule matches several elements.
       // The first element added to the rule tree will add that rule, subsequent
       // ones will just reuse it (if the path so far in the rule tree has
@@ -206,7 +209,9 @@ export namespace RuleTree {
       if (
         // We cannot simply test === between the .selector because we do not
         // want to identify two null.
-        (this._block.selector ?? 0) === (block.selector ?? 1)
+        (this._block.selector ?? 0) === (block.selector ?? 1) &&
+        Precedence.isImportant(this._block.precedence) ===
+          Precedence.isImportant(block.precedence)
       ) {
         return this;
       }

packages/alfa-cascade/test/rule-tree.spec.ts

@@ -247,3 +247,27 @@ test(".add() sort items by precedence", (t) => {
     },
   ]);
 });
+
+test(".add() adds several block when a rule has both normal and important declarations", (t) => {
+  const blocks = (
+    Block.from(
+      h.rule.style("div", { foo: "bar", hello: "world !important" }),
+      0,
+      1,
+      {
+        normal: Layer.of("", false).withOrder(0),
+        important: Layer.of("", true).withOrder(0),
+      },
+    )[0] as Array<Block<Block.Source, true>>
+  ).sort(Block.compare);
+
+  const tree = RuleTree.empty();
+  tree.add(blocks);
+
+  t.deepEqual(tree.toJSON(), [
+    {
+      block: blocks[0].toJSON(),
+      children: [{ block: blocks[1].toJSON(), children: [] }],
+    },
+  ]);
+});

packages/alfa-style/src/style.ts

@@ -322,8 +322,6 @@ export namespace Style {
           // (highest precedence rules are at the bottom), thus the declarations
           // are seen in decreasing precedence and pushed to the end of the
           // existing list which is thus also ordered in decreasing precedence.
-          // Cascade doesn't handle importance of declaration, hence this will
-          // still have to be done here (through `shouldOverride`).
           for (const node of cascade
             .get(element, context)
             .inclusiveAncestors()) {