|
| 1 | +""" |
| 2 | +Regression test for the news crop aspect-ratio contract. |
| 3 | +
|
| 4 | +Bug this pins |
| 5 | +------------- |
| 6 | +A ``NewsItem.cropping`` is an ``ImageRatioField`` locked to |
| 7 | +``NEWS_THUMBNAIL_SIZE`` (500x300 == 5:3). The admin Cropper.js widget lets an |
| 8 | +editor pick a 5:3 crop box, stored as an "x1,y1,x2,y2" string. When a template |
| 9 | +renders ``{% thumbnail news_item.image <W>x<H> box=news_item.cropping crop=True %}`` |
| 10 | +easy_thumbnails first applies that 5:3 box (via ``crop_corners``) and THEN, when |
| 11 | +``<W>x<H>`` is a *different* aspect ratio, applies a SECOND center-crop to force |
| 12 | +the target ratio -- silently trimming the top/bottom of the editor's chosen crop. |
| 13 | +
|
| 14 | +That second crop is invisible in the admin (the preview only ever shows the 5:3 |
| 15 | +box), so it shipped a head-clipping bug: the news *detail* page rendered at |
| 16 | +750x350 (15:7) against a 5:3 crop and lopped off people's heads. |
| 17 | +
|
| 18 | +The durable fix is a rule: **every on-page crop render of a news image must use |
| 19 | +the crop field's aspect ratio; only the pixel size may vary.** This test pins |
| 20 | +that rule by scanning the templates, so a future size change to a mismatching |
| 21 | +ratio fails here instead of silently re-cropping in production. |
| 22 | +
|
| 23 | +Intentional exceptions (a single stored crop box genuinely cannot be WYSIWYG at |
| 24 | +these ratios) are listed in ``ALLOWED_EXCEPTIONS`` with the reason. |
| 25 | +""" |
| 26 | + |
| 27 | +import re |
| 28 | +from pathlib import Path |
| 29 | + |
| 30 | +from django.test import SimpleTestCase |
| 31 | + |
| 32 | +from website.models.news import NEWS_THUMBNAIL_SIZE |
| 33 | + |
| 34 | +# Directory holding this app's templates. |
| 35 | +TEMPLATES_DIR = Path(__file__).resolve().parent.parent / "templates" |
| 36 | + |
| 37 | +# Image sources whose renders must honor the news crop aspect ratio. |
| 38 | +NEWS_IMAGE_SOURCES = {"news_item.image", "recent_news_item.image"} |
| 39 | + |
| 40 | +# The crop editor / ImageRatioField aspect ratio (width / height). |
| 41 | +NEWS_ASPECT = NEWS_THUMBNAIL_SIZE[0] / NEWS_THUMBNAIL_SIZE[1] |
| 42 | + |
| 43 | +# Floating-point slack when comparing aspect ratios. |
| 44 | +ASPECT_TOLERANCE = 0.01 |
| 45 | + |
| 46 | +# (width, height) render sizes that intentionally do NOT match the crop ratio, |
| 47 | +# each with the reason it is exempt. A single stored 5:3 crop box cannot be |
| 48 | +# WYSIWYG at these ratios, and that is acceptable here: |
| 49 | +ALLOWED_EXCEPTIONS = { |
| 50 | + (1200, 630): ( |
| 51 | + "Open Graph / social share card. 1200x630 (~1.91:1) is the platform " |
| 52 | + "standard; social sites re-crop to ~1.91:1 on their end regardless of " |
| 53 | + "what we send, so a single 5:3 crop box cannot be WYSIWYG for both the " |
| 54 | + "page and the social card. True fix would need a dedicated social crop." |
| 55 | + ), |
| 56 | + (50, 50): ( |
| 57 | + "Round sidebar avatar chip. The CSS (.news-sidebar-image) forces a " |
| 58 | + "50x50 circle via object-fit:cover + border-radius:full, so the render " |
| 59 | + "is a deliberate decorative crop, not a WYSIWYG content image." |
| 60 | + ), |
| 61 | +} |
| 62 | + |
| 63 | +# Matches: {% thumbnail <src> <WxH> ...rest... %} |
| 64 | +# <src> is the first positional arg (e.g. news_item.image); <size> is the |
| 65 | +# WxH token, optionally single- or double-quoted; <rest> is everything up to %}. |
| 66 | +THUMBNAIL_TAG_RE = re.compile( |
| 67 | + r"{%\s*thumbnail\s+(?P<src>[\w.]+)\s+" |
| 68 | + r"['\"]?(?P<w>\d+)x(?P<h>\d+)['\"]?" |
| 69 | + r"(?P<rest>[^%]*?)%}" |
| 70 | +) |
| 71 | + |
| 72 | + |
| 73 | +class NewsCropAspectRatioTests(SimpleTestCase): |
| 74 | + """Every cropped on-page render of a news image must match the crop ratio.""" |
| 75 | + |
| 76 | + def _news_crop_renders(self): |
| 77 | + """ |
| 78 | + Yield (template_path, w, h, tag) for every ``{% thumbnail %}`` tag in the |
| 79 | + app templates that renders a news image WITH a crop box and ``crop`` on. |
| 80 | +
|
| 81 | + Only ``crop``-enabled renders can trigger the second center-crop; a |
| 82 | + non-crop render scales-to-fit and leaves the editor's box intact, so we |
| 83 | + deliberately skip those. |
| 84 | + """ |
| 85 | + for template in TEMPLATES_DIR.rglob("*.html"): |
| 86 | + text = template.read_text(encoding="utf-8") |
| 87 | + for m in THUMBNAIL_TAG_RE.finditer(text): |
| 88 | + if m.group("src") not in NEWS_IMAGE_SOURCES: |
| 89 | + continue |
| 90 | + rest = m.group("rest") |
| 91 | + if "box=" not in rest: |
| 92 | + continue # no crop box -> nothing to keep WYSIWYG |
| 93 | + if not re.search(r"\bcrop\b", rest): |
| 94 | + continue # scale-to-fit, no second crop |
| 95 | + yield ( |
| 96 | + template.relative_to(TEMPLATES_DIR), |
| 97 | + int(m.group("w")), |
| 98 | + int(m.group("h")), |
| 99 | + m.group(0), |
| 100 | + ) |
| 101 | + |
| 102 | + def test_scan_finds_the_detail_render(self): |
| 103 | + """Guard against the regex silently matching nothing (false pass).""" |
| 104 | + sizes = {(w, h) for _, w, h, _ in self._news_crop_renders()} |
| 105 | + self.assertIn( |
| 106 | + (750, 450), |
| 107 | + sizes, |
| 108 | + "Expected the news detail render (750x450) to be present. If the " |
| 109 | + "size changed, update this test AND confirm it still matches the " |
| 110 | + f"5:3 crop ratio. Found sizes: {sorted(sizes)}", |
| 111 | + ) |
| 112 | + |
| 113 | + def test_on_page_news_renders_match_crop_aspect(self): |
| 114 | + mismatches = [] |
| 115 | + for template, w, h, tag in self._news_crop_renders(): |
| 116 | + if (w, h) in ALLOWED_EXCEPTIONS: |
| 117 | + continue |
| 118 | + aspect = w / h |
| 119 | + if abs(aspect - NEWS_ASPECT) > ASPECT_TOLERANCE: |
| 120 | + mismatches.append( |
| 121 | + f" {template}: {w}x{h} (ratio {aspect:.3f}) != crop ratio " |
| 122 | + f"{NEWS_ASPECT:.3f}\n {tag.strip()}" |
| 123 | + ) |
| 124 | + |
| 125 | + self.assertFalse( |
| 126 | + mismatches, |
| 127 | + "These news image renders use a crop box but a target size whose " |
| 128 | + "aspect ratio differs from the crop editor's " |
| 129 | + f"({NEWS_THUMBNAIL_SIZE[0]}x{NEWS_THUMBNAIL_SIZE[1]} == " |
| 130 | + f"{NEWS_ASPECT:.3f}). easy_thumbnails will center-crop the editor's " |
| 131 | + "box a second time, trimming top/bottom (clipped heads). Fix the " |
| 132 | + "size to share the crop ratio, or add it to ALLOWED_EXCEPTIONS with " |
| 133 | + "a reason:\n" + "\n".join(mismatches), |
| 134 | + ) |
0 commit comments