Skip to content

Commit 76d254b

Browse files
authored
Stop showing invisible content in HTML diff (#43)
This also solves an issue where text coloring would be incorrect and sometimes hard to read in Safari. This change is purely about the additional styling we add to diffs -- rather than use `all: unset` in our CSS, we take a more precise approach and only reset certain styles, and do not apply most of those resets to the descendents of the `<ins>`/`<del>` elements (we only apply them to the elements themselves). Fixes #24.
1 parent 829c790 commit 76d254b

File tree

2 files changed

+29
-10
lines changed

2 files changed

+29
-10
lines changed

docs/source/release-history.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ In Development
1111

1212
- Improve handling of lazy-loaded images in :func:`web_monitoring_diff.html_diff_render`. When images are lazy-loaded via JS, they usually use the ``data-src`` or ``data-srcset`` attributes, and we now check those, too. Additionally, if two images have no detectable URLs, we now treat them as the same, rather than different. (`#37 <https://github.com/edgi-govdata-archiving/web-monitoring-diff/issues/37>`_)
1313

14+
- Stop showing inline scripts and styles in :func:`web_monitoring_diff.html_diff_render`. These still get wrapped with ``<del>`` or ``<ins>`` elements, but they don’t show up visually since they aren’t elements that should be visually rendered. (`#24 <https://github.com/edgi-govdata-archiving/web-monitoring-diff/issues/24>`_
15+
1416

1517
Version 0.1.0
1618
-------------

web_monitoring_diff/html_render_diff.py

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -425,16 +425,7 @@ def html_diff_render(a_text, b_text, a_headers=None, b_headers=None,
425425
"style",
426426
type="text/css",
427427
id='wm-diff-style')
428-
429-
color_palette = get_color_palette()
430-
change_styles.string = f'''
431-
ins.wm-diff, ins.wm-diff > * {{background-color:
432-
{color_palette['differ_insertion']} !important;
433-
all: unset;}}
434-
del.wm-diff, del.wm-diff > * {{background-color:
435-
{color_palette['differ_deletion']} !important;
436-
all: unset;}}
437-
script {{display: none !important;}}'''
428+
change_styles.string = get_diff_styles()
438429
soup.head.append(change_styles)
439430

440431
soup.body.replace_with(diff_body)
@@ -1864,6 +1855,32 @@ def get_matching_blocks(self):
18641855
or not item[2]]
18651856

18661857

1858+
def get_diff_styles():
1859+
colors = get_color_palette()
1860+
# Unset local `<ins>`/`<del>` styling on the page that might clash with
1861+
# our diff elements. Note that `all: unset` has browser bugs that are
1862+
# problematic (e.g. https://bugs.webkit.org/show_bug.cgi?id=158782) so
1863+
# we need to use a list of specific properties we're concerned about
1864+
# instead. (It can also cause the contents of `<style>` and `<script>`
1865+
# tags to be rendered on the page, which is also bad.)
1866+
return f'''
1867+
ins.wm-diff, del.wm-diff {{
1868+
display: unset;
1869+
visibility: unset;
1870+
opacity: 1;
1871+
clip: auto;
1872+
text-decoration: unset;
1873+
color: inherit;
1874+
}}
1875+
ins.wm-diff, ins.wm-diff > * {{
1876+
background-color: {colors['differ_insertion']} !important;
1877+
}}
1878+
del.wm-diff, del.wm-diff > * {{
1879+
background-color: {colors['differ_deletion']} !important;
1880+
}}
1881+
script {{display: none !important;}}'''
1882+
1883+
18671884
UPDATE_CONTRAST_SCRIPT = """
18681885
(function () {
18691886
// Update the text color of change elements to ensure a readable level

0 commit comments

Comments
 (0)