mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-17 16:46:31 +03:00
* Fix: detect placeholder contrast (#790) `detect` never read `::placeholder` color, so pale placeholders passed. Score them with the same WCAG math as body text, without host class/clip heuristics. Prepared with AI assistance. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix: match descendant ::placeholder hosts (#790) `.form ::placeholder` kept the ancestor as the host. Reuse the hover combinator star-fill so the color lands on the inputs inside. Prepared with AI assistance. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix: placeholder-shown and gradient alpha (#790) Browser scans skip when :placeholder-shown is false, so a live filled field does not keep the HTML value attribute's empty state. Translucent placeholders flatten over each gradient stop before scoring. Prepared with AI assistance. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix: trailing combinator only for ::placeholder hosts (#790) `star_empty_compounds` turned `.label + ::placeholder` into `.label *+*`. Fill only a trailing empty compound so adjacent-sibling hosts still match. Prepared with AI assistance. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -20,9 +20,9 @@ use impeccable_core::checks::measures::{
|
||||
use impeccable_core::checks::rules::{
|
||||
check_borders, check_colors, check_glow, check_hero_eyebrow, check_hover_contrast,
|
||||
check_icon_tile, check_italic_serif, check_kicker_above_heading, check_motion,
|
||||
is_emoji_only_text, is_heading_tag, resolve_hero_heading_size_px, BorderOpts, ColorOpts,
|
||||
GlowOpts, HeroEyebrowOpts, HoverContrastOpts, IconTileOpts, ItalicSerifOpts, KickerCandidate,
|
||||
MotionOpts, RuleHit, Sides,
|
||||
check_placeholder_colors, is_emoji_only_text, is_heading_tag, resolve_hero_heading_size_px,
|
||||
BorderOpts, ColorOpts, GlowOpts, HeroEyebrowOpts, HoverContrastOpts, IconTileOpts,
|
||||
ItalicSerifOpts, KickerCandidate, MotionOpts, RuleHit, Sides,
|
||||
};
|
||||
use impeccable_core::checks::text_rules::{
|
||||
check_numbered_section_labels, is_kicker_candidate, is_numbered_section_label_candidate,
|
||||
@@ -539,7 +539,7 @@ pub fn check_element_colors(
|
||||
sv(style, "backgroundClip")
|
||||
}
|
||||
};
|
||||
check_colors(&ColorOpts {
|
||||
let color_opts = ColorOpts {
|
||||
tag: tag.to_string(),
|
||||
text_color,
|
||||
bg_color: own_bg,
|
||||
@@ -557,7 +557,43 @@ pub fn check_element_colors(
|
||||
bg_image: Some(sv(style, "backgroundImage").to_string()),
|
||||
class_list: Some(el.class_name().to_string()),
|
||||
detector_is_browser: false,
|
||||
})
|
||||
};
|
||||
let mut findings = check_colors(&color_opts);
|
||||
if tag == "input" || tag == "textarea" {
|
||||
let placeholder = el.get_attribute("placeholder").unwrap_or("").trim();
|
||||
if !placeholder.is_empty() {
|
||||
let skip = if tag == "input" {
|
||||
let t = js::to_lower_case(el.get_attribute("type").unwrap_or("text"));
|
||||
matches!(
|
||||
t.as_str(),
|
||||
"hidden" | "checkbox" | "radio" | "file" | "submit" | "button" | "image"
|
||||
| "reset" | "range" | "color"
|
||||
) || el
|
||||
.get_attribute("value")
|
||||
.is_some_and(|v| !js::trim(v).is_empty())
|
||||
} else {
|
||||
!js::trim(&direct_text).is_empty()
|
||||
};
|
||||
if !skip {
|
||||
if let Some(ph_style) = el.doc.get_placeholder_style(el.id()) {
|
||||
let ph_color = custom_props
|
||||
.and_then(|m| {
|
||||
measures::parse_color_resolved(sv_opt(ph_style, "color"), Some(m))
|
||||
})
|
||||
.or_else(|| parse_rgb(sv_opt(ph_style, "color")))
|
||||
.or_else(|| parse_any_color(sv_opt(ph_style, "color")));
|
||||
if let Some(ph_color) = ph_color {
|
||||
findings.extend(check_placeholder_colors(
|
||||
&color_opts,
|
||||
placeholder,
|
||||
ph_color,
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
findings
|
||||
}
|
||||
|
||||
/// JS: checks.mjs#checkElementHoverContrast(el, style, tag, window)
|
||||
|
||||
@@ -139,6 +139,31 @@ static PSEUDO_RULE_RE: Lazy<Regex> = Lazy::new(|| {
|
||||
))
|
||||
.expect("PSEUDO_RULE_RE")
|
||||
});
|
||||
static PLACEHOLDER_RULE_RE: Lazy<Regex> = Lazy::new(|| {
|
||||
Regex::new(r"(?i)^(.*)(?:::placeholder|::?-webkit-input-placeholder|::?-moz-placeholder)$")
|
||||
.expect("PLACEHOLDER_RULE_RE")
|
||||
});
|
||||
|
||||
fn placeholder_host_selector(selector: &str) -> Option<String> {
|
||||
let pm = PLACEHOLDER_RULE_RE.captures(selector)?;
|
||||
let captured = pm.get(1).map(|m| m.as_str()).unwrap_or("");
|
||||
let trimmed_end = captured.trim_end_matches(|c: char| js::is_js_whitespace(c));
|
||||
if trimmed_end.is_empty() {
|
||||
return Some("*".to_string());
|
||||
}
|
||||
// Only fill a trailing empty compound. `star_empty_compounds` would
|
||||
// rewrite `.label + ::placeholder` to `.label *+*`.
|
||||
let last = trimmed_end.chars().last().unwrap();
|
||||
if captured.len() != trimmed_end.len() || last == '>' || last == '+' || last == '~' {
|
||||
if last == '>' || last == '+' || last == '~' {
|
||||
Some(format!("{}*", trimmed_end))
|
||||
} else {
|
||||
Some(format!("{} *", trimmed_end))
|
||||
}
|
||||
} else {
|
||||
Some(trimmed_end.to_string())
|
||||
}
|
||||
}
|
||||
static COLOR_TOKEN_RE: Lazy<Regex> = Lazy::new(|| {
|
||||
Regex::new(r"(?i)(?:rgba?|hsla?|oklch|oklab|lab|lch|hwb|color-mix)\([^)]*(?:\([^)]*\))?[^)]*\)|#[0-9a-f]{3,8}(?-u:\b)")
|
||||
.expect("COLOR_TOKEN_RE")
|
||||
@@ -252,6 +277,7 @@ pub fn build_static_style_map(
|
||||
) {
|
||||
let mut specified: SpecifiedStore<NodeId> = SpecifiedStore::new();
|
||||
let mut hover_specified: SpecifiedStore<NodeId> = SpecifiedStore::new();
|
||||
let mut placeholder_specified: SpecifiedStore<NodeId> = SpecifiedStore::new();
|
||||
let root_custom_props = collect_css_custom_props(css_text);
|
||||
let rules = profile::step(
|
||||
profile,
|
||||
@@ -264,7 +290,12 @@ pub fn build_static_style_map(
|
||||
Meta::new("selector-match", "css-selectors", file_path),
|
||||
|| {
|
||||
for rule in &rules {
|
||||
if !rule.is_hover {
|
||||
let placeholder_host = if rule.is_hover {
|
||||
None
|
||||
} else {
|
||||
placeholder_host_selector(&rule.selector)
|
||||
};
|
||||
if !rule.is_hover && placeholder_host.is_none() {
|
||||
if let Some(pm) = PSEUDO_RULE_RE.captures(&rule.selector) {
|
||||
let base = pm.get(1).map(|m| m.as_str()).unwrap_or("").to_string();
|
||||
mark_pseudo_rule(doc, rule, &base, &root_custom_props);
|
||||
@@ -273,6 +304,8 @@ pub fn build_static_style_map(
|
||||
}
|
||||
let match_selector: Option<&str> = if rule.is_hover {
|
||||
rule.match_selector.as_deref()
|
||||
} else if let Some(ref host) = placeholder_host {
|
||||
Some(host.as_str())
|
||||
} else {
|
||||
Some(rule.selector.as_str())
|
||||
};
|
||||
@@ -296,11 +329,18 @@ pub fn build_static_style_map(
|
||||
};
|
||||
let store = if rule.is_hover {
|
||||
&mut hover_specified
|
||||
} else if placeholder_host.is_some() {
|
||||
&mut placeholder_specified
|
||||
} else {
|
||||
&mut specified
|
||||
};
|
||||
for node in matched {
|
||||
for decl in &rule.declarations {
|
||||
if placeholder_host.is_some()
|
||||
&& js::to_lower_case(&decl.prop) != "color"
|
||||
{
|
||||
continue;
|
||||
}
|
||||
let meta = DeclMeta {
|
||||
important: decl.important,
|
||||
specificity: rule.specificity,
|
||||
@@ -341,7 +381,7 @@ pub fn build_static_style_map(
|
||||
profile,
|
||||
Meta::new("cascade", "compute-styles", file_path),
|
||||
|| {
|
||||
compute_styles(doc, &specified, &hover_specified);
|
||||
compute_styles(doc, &specified, &hover_specified, &placeholder_specified);
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -353,6 +393,7 @@ fn compute_styles(
|
||||
doc: &mut StaticDocument,
|
||||
specified: &SpecifiedStore<NodeId>,
|
||||
hover_specified: &SpecifiedStore<NodeId>,
|
||||
placeholder_specified: &SpecifiedStore<NodeId>,
|
||||
) {
|
||||
let mut computed: HashMap<NodeId, Rc<StyleValues>> = HashMap::new();
|
||||
let mut customs: HashMap<NodeId, Rc<CustomProps>> = HashMap::new();
|
||||
@@ -368,6 +409,7 @@ fn compute_styles(
|
||||
.map(|e| (e.id(), None))
|
||||
.collect();
|
||||
let mut hover_out: Vec<(NodeId, StyleValues)> = Vec::new();
|
||||
let mut placeholder_out: Vec<(NodeId, StyleValues)> = Vec::new();
|
||||
|
||||
while let Some((node, parent)) = stack.pop() {
|
||||
let parent_style: Option<Rc<StyleValues>> = parent.and_then(|p| computed.get(&p).cloned());
|
||||
@@ -439,6 +481,21 @@ fn compute_styles(
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(ph_map) = placeholder_specified.get(&node) {
|
||||
if let Some(color_decl) = ph_map.get("color") {
|
||||
let next = normalize_static_css_value(
|
||||
"color",
|
||||
&color_decl.value,
|
||||
&custom_props,
|
||||
Some(&values),
|
||||
Some(&values),
|
||||
);
|
||||
let mut ph_style = StyleValues::default();
|
||||
ph_style.insert("color".to_string(), next);
|
||||
placeholder_out.push((node, ph_style));
|
||||
}
|
||||
}
|
||||
|
||||
let style_rc = Rc::new(values);
|
||||
computed.insert(node, style_rc);
|
||||
customs.insert(node, Rc::new(custom_props));
|
||||
@@ -460,6 +517,9 @@ fn compute_styles(
|
||||
for (node, style) in hover_out {
|
||||
doc.set_hover_style(node, style);
|
||||
}
|
||||
for (node, style) in placeholder_out {
|
||||
doc.set_placeholder_style(node, style);
|
||||
}
|
||||
}
|
||||
|
||||
/// `STATIC_DEFAULT_STYLE[prop]` lookup re-exported for the adapters.
|
||||
|
||||
@@ -55,6 +55,7 @@ pub struct StaticDocument {
|
||||
pub html: Html,
|
||||
styles: HashMap<NodeId, StyleValues>,
|
||||
hover_styles: HashMap<NodeId, StyleValues>,
|
||||
placeholder_styles: HashMap<NodeId, StyleValues>,
|
||||
accent_dash: HashSet<NodeId>,
|
||||
pseudo_surface: HashMap<NodeId, Rgba>,
|
||||
selector_cache: RefCell<HashMap<String, Result<Selector, SelectorError>>>,
|
||||
@@ -174,6 +175,7 @@ impl StaticDocument {
|
||||
html,
|
||||
styles: HashMap::new(),
|
||||
hover_styles: HashMap::new(),
|
||||
placeholder_styles: HashMap::new(),
|
||||
accent_dash: HashSet::new(),
|
||||
pseudo_surface: HashMap::new(),
|
||||
selector_cache: RefCell::new(HashMap::new()),
|
||||
@@ -319,6 +321,12 @@ impl StaticDocument {
|
||||
pub fn get_hover_style(&self, node: NodeId) -> Option<&StyleValues> {
|
||||
self.hover_styles.get(&node)
|
||||
}
|
||||
pub fn set_placeholder_style(&mut self, node: NodeId, style: StyleValues) {
|
||||
self.placeholder_styles.insert(node, style);
|
||||
}
|
||||
pub fn get_placeholder_style(&self, node: NodeId) -> Option<&StyleValues> {
|
||||
self.placeholder_styles.get(&node)
|
||||
}
|
||||
pub fn set_accent_dash_pseudo(&mut self, node: NodeId) {
|
||||
self.accent_dash.insert(node);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
//! Integration tests for `::placeholder` contrast detection (#790).
|
||||
|
||||
use impeccable_html::{detect_html_source, DetectHtmlOptions};
|
||||
use std::path::Path;
|
||||
|
||||
const ISSUER_REPRO: &str = r#"<!DOCTYPE html>
|
||||
<html><head><style>
|
||||
input::placeholder { color: #bbbbbb; }
|
||||
input { background: white; font-size: 16px; width: 200px; height: 40px; border: 1px solid #ccc; padding: 8px; box-sizing: border-box; }
|
||||
</style></head>
|
||||
<body><input placeholder="Search"></body></html>
|
||||
"#;
|
||||
|
||||
fn scan(html: &str) -> Vec<impeccable_core::findings::Finding> {
|
||||
detect_html_source(html, Path::new("/tmp/placeholder.html"), &DetectHtmlOptions::default())
|
||||
}
|
||||
|
||||
fn repo_root() -> std::path::PathBuf {
|
||||
std::env::var("IMPECCABLE_PUBLIC_REPO")
|
||||
.map(std::path::PathBuf::from)
|
||||
.unwrap_or_else(|_| Path::new(env!("CARGO_MANIFEST_DIR")).join("../.."))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn issuer_repro_flags_pale_placeholder() {
|
||||
let findings = scan(ISSUER_REPRO);
|
||||
assert!(
|
||||
findings.iter().any(|f| f.antipattern == "low-contrast"),
|
||||
"expected low-contrast finding, got {findings:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bare_placeholder_selector_flags() {
|
||||
let html = r#"<!DOCTYPE html>
|
||||
<html><head><style>
|
||||
::placeholder { color: #bbbbbb; }
|
||||
input { background: white; font-size: 16px; width: 200px; height: 40px; }
|
||||
</style></head>
|
||||
<body><input placeholder="Search"></body></html>
|
||||
"#;
|
||||
let findings = scan(html);
|
||||
assert!(
|
||||
findings.iter().any(|f| f.antipattern == "low-contrast"),
|
||||
"expected low-contrast for bare ::placeholder, got {findings:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn descendant_placeholder_selector_flags() {
|
||||
let html = r#"<!DOCTYPE html>
|
||||
<html><head><style>
|
||||
.form ::placeholder { color: #bbbbbb; }
|
||||
input { background: white; font-size: 16px; width: 200px; height: 40px; }
|
||||
</style></head>
|
||||
<body><div class="form"><input placeholder="Search"></div></body></html>
|
||||
"#;
|
||||
let findings = scan(html);
|
||||
assert!(
|
||||
findings.iter().any(|f| f.antipattern == "low-contrast"),
|
||||
"expected low-contrast for descendant ::placeholder, got {findings:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sibling_placeholder_selector_flags() {
|
||||
let html = r#"<!DOCTYPE html>
|
||||
<html><head><style>
|
||||
.label + ::placeholder { color: #bbbbbb; }
|
||||
input { background: white; font-size: 16px; width: 200px; height: 40px; }
|
||||
</style></head>
|
||||
<body><label class="label">Name</label><input placeholder="Search"></body></html>
|
||||
"#;
|
||||
let findings = scan(html);
|
||||
assert!(
|
||||
findings.iter().any(|f| f.antipattern == "low-contrast"),
|
||||
"expected low-contrast for sibling ::placeholder, got {findings:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fixture_flag_and_pass_cases() {
|
||||
let fixture = repo_root().join("tests/fixtures/antipatterns/placeholder-contrast.html");
|
||||
assert!(
|
||||
fixture.is_file(),
|
||||
"missing fixture at {}",
|
||||
fixture.display()
|
||||
);
|
||||
let html = std::fs::read_to_string(&fixture).unwrap();
|
||||
let findings = detect_html_source(&html, &fixture, &DetectHtmlOptions::default());
|
||||
let ids: Vec<&str> = findings.iter().map(|f| f.antipattern.as_str()).collect();
|
||||
let snippets: Vec<&str> = findings.iter().map(|f| f.snippet.as_str()).collect();
|
||||
|
||||
for needle in [
|
||||
"Pale Placeholder On White Field",
|
||||
"Pale Placeholder On White Textarea",
|
||||
"Translucent Placeholder On Light Field",
|
||||
"Pale Placeholder On Frosted Panel",
|
||||
] {
|
||||
assert!(
|
||||
snippets.iter().any(|s| s.contains(needle)),
|
||||
"expected flag for placeholder {needle:?}, findings={findings:?}"
|
||||
);
|
||||
}
|
||||
|
||||
for needle in [
|
||||
"Ink Placeholder On White Field",
|
||||
"Light Placeholder On Dark Field",
|
||||
"Filled Field Hides Placeholder",
|
||||
"Unstyled Placeholder Uses UA Color",
|
||||
] {
|
||||
assert!(
|
||||
!snippets.iter().any(|s| s.contains(needle)),
|
||||
"pass case {needle:?} should not flag, findings={findings:?}"
|
||||
);
|
||||
}
|
||||
|
||||
assert!(
|
||||
ids.iter().filter(|id| **id == "low-contrast").count() >= 4,
|
||||
"expected at least four low-contrast hits, got {findings:?}"
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user