Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browser Prefix Transforms are Over-Eager #328

Open
ryan-richt opened this issue Feb 27, 2022 · 0 comments
Open

Browser Prefix Transforms are Over-Eager #328

ryan-richt opened this issue Feb 27, 2022 · 0 comments

Comments

@ryan-richt
Copy link

ryan-richt commented Feb 27, 2022

I could be totally wrong, or wrong on the intent of the current code, but I think there might be 2 small bugs that lead to over-eager browser prefixing.

  1. Agent equality does not include the agent name, so the Subject support maps are immediately reduced by many key collisions upon construction
  2. Transform.prefix looks up the relevant prefixes in a lossy way (by reducing both the lookup map and the key in question to their prefixes instead of holding on to their Agents) that computes the need for more prefixes than necessary

One (possibly wrong) way to fix is:

Fix (1)

final case class Agent(prefix: Prefix, prefixExceptions: Map[VerStr, Prefix])

Alter ^ this definition to include the agent name e.g.:
final case class Agent(name: String, prefix: Prefix, prefixExceptions: Map[VerStr, Prefix])

which is accomplished by changing the generator code here:

final case class Agent(prefix: Prefix, prefixExceptions: Map[VerStr, Prefix])

as well as the generated instances here:

s"""val $key = Agent($p2, ${fmtmap(fmtstr, identity[String])(prefixExceptions)})"""

Fix (2)

Change

private def prefix[R](subject: Subject, pa: PrefixApply, l: CssKV.Lens): Transform = {
import CanIUse2._
val pp0 = prefixPlan(subject)
Transform(e => {
val pp = filteredPrefixPlan(e.prefixWhitelist, pp0)
kv => runPlan(pp, pa, l, kv)
})
}

to

  private def prefix[R](subject: Subject, pa: PrefixApply, l: CssKV.Lens): Transform = {
    import CanIUse2._
    Transform(e => {
      val platformAgents: Set[CanIUse.Agent] = CanIUse2.agentsForPlatform(e.platform).toSet
      val pp: PrefixPlan = prefixPlan(subject.filter{ case (k, v) => platformAgents(k) })
      kv => runPlan(pp, pa, l, kv)
    })
  }

Because the current code looks up pp0 based on all possible Agents. Then it uses e.prefixWhitelist which is the (lossy) set of all prefixes aka engines (e.g. webkit) collapsed across all Agents for this platform, to filter down the list of all possible Agents from pp0, based on their prefixes aka engines. This lossy lookup means that extra browser prefixes will be emitted even with the Platform does not intend to support a specific Agent, but because that non-supported Agent shares a prefix with another Agent that DOES NOT support the feature in question, we calculate that the intended Agent also does not support that feature even if it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant