CSS behaviors bug report: dark/light switch non-functional

CSS behaviors bug report: dark/light switch non-functional

[Robin H. Johnson]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

I wanted to include a patch, but no obvious way without it being messy
really stood out, so this is only a bug report.

contrib/css describes using the 'prefers-color-scheme' media query
selector, but Gentoo got a report that the selection between light &
dark didn't work.

One of the other devs traced it down to nuances of the HTML link media
attribute: query expressions are not valid in the HTML variant in some
browsers.

This is also mentioned here:
https://stackoverflow.com/questions/69113672/loading-css-based-on-prefers-color-scheme-media-query
> Loading CSS based on any media query expression seems to be out of specification for the media attribute.

As a workaround, I merged the two files with a wrapper, but it feels
sub-optimal, because the entire text gets injected into the PI HTML.

```
@media(prefers-color-scheme: dark) {
 ... #content of 216dark
}
@media(prefers-color-scheme: light) {
 ... #content of 216light
}
```

I think the cleanest solution would be a variant of the above:
clauses, but with @import statements inside them, and making PI-httpd
expose all the CSS Files.

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation President & Treasurer
E-Mail   : robbat2@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 1113 bytes --]

Re: CSS behaviors bug report: dark/light switch non-functional

[Eric Wong]

"Robin H. Johnson" <robbat2@gentoo.org> wrote:
> I wanted to include a patch, but no obvious way without it being messy
> really stood out, so this is only a bug report.

Yeah, I've been taking a stab at this in between other things but
not really making progress.
 
> contrib/css describes using the 'prefers-color-scheme' media query
> selector, but Gentoo got a report that the selection between light &
> dark didn't work.

I'm not even sure how that works anymore with fingerprint resistance.

Which browsers + versions + configs are they using?

Will need some testing from others here...

> One of the other devs traced it down to nuances of the HTML link media
> attribute: query expressions are not valid in the HTML variant in some
> browsers.

OK

> This is also mentioned here:
> https://stackoverflow.com/questions/69113672/loading-css-based-on-prefers-color-scheme-media-query
> > Loading CSS based on any media query expression seems to be out of specification for the media attribute.
> 
> As a workaround, I merged the two files with a wrapper, but it feels
> sub-optimal, because the entire text gets injected into the PI HTML.
> 
> ```
> @media(prefers-color-scheme: dark) {
>  ... #content of 216dark
> }
> @media(prefers-color-scheme: light) {
>  ... #content of 216light
> }
> ```

Yeah.

> I think the cleanest solution would be a variant of the above:
> clauses, but with @import statements inside them, and making PI-httpd
> expose all the CSS Files.

I think a safe way is to allow loading any CSS files present in
the same directory as an admin-specified CSS file.  IOW,
	public-inbox.css = /path/to/foo/216dark.css

Will allow autoloading all /path/to/foo/*.css files.


Also, I don't think @import works inside @media,
https://developer.mozilla.org/en-US/docs/Web/CSS/@import says:

| The @import CSS at-rule is used to import style rules from other valid
| stylesheets. An @import rule must be defined at the top of the stylesheet,
| before any other at-rule (except @charset and @layer) and style declarations,
| or it will be ignored.

So I think it'd have to be something like:

@import url('216dark.css') screen and (prefers-color-scheme: light)

If I'm understanding that correctly.

Hopefully I'll be back in a few hours or tomorrow with something
to test...

[PATCH 0/5] css improvements (hopefully)

[Eric Wong]

1 and 2 are simple cleanups and an optimization to make
the resulting strace more pleasant.

3 and 4 should address Robin's bug report but it requires
setting the `load=import' attribute (see manpage update)

mtime in 5 is something that we should've been doing all along.

Eric Wong (5):
  user_content: simplify internal API and use v5.12
  www: don't reread CSS files
  www: load CSS files in same dir if @import is in use
  www: allow specifying CSS @import or <link> tags
  www: use mtime as CSS cache-buster instead of ctime

 Documentation/public-inbox-config.pod |  18 ++--
 lib/PublicInbox/UserContent.pm        |  30 ++++---
 lib/PublicInbox/WWW.pm                | 122 ++++++++++++++++++++------
 lib/PublicInbox/WwwText.pm            |   6 +-
 4 files changed, 124 insertions(+), 52 deletions(-)

[PATCH 1/5] user_content: simplify internal API and use v5.12

[Eric Wong]

We use {env} and {ibx} everywhere so there's no point in
unpacking args.  There's no odd unicode_strings problems
here, either, so we can use v5.12 and autodie to reduce
`or die' checks.
---
 lib/PublicInbox/UserContent.pm | 27 ++++++++++++++-------------
 lib/PublicInbox/WWW.pm         |  3 +--
 lib/PublicInbox/WwwText.pm     |  6 ++----
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/lib/PublicInbox/UserContent.pm b/lib/PublicInbox/UserContent.pm
index f28610f7..9dacfa0b 100644
--- a/lib/PublicInbox/UserContent.pm
+++ b/lib/PublicInbox/UserContent.pm
@@ -1,11 +1,11 @@
-# Copyright (C) 2019-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # Self-updating module containing a sample CSS for client-side
 # customization by users of public-inbox.  Used by Makefile.PL
 package PublicInbox::UserContent;
-use strict;
-use warnings;
+use v5.12;
+use autodie qw(close open seek);
 
 # this sub is updated automatically:
 sub CSS () {
@@ -71,9 +71,9 @@ _
 # end of auto-updated sub
 
 # return a sample CSS
-sub sample ($$) {
-	my ($ibx, $env) = @_;
-	my $url_prefix = $ibx->base_url($env);
+sub sample ($) {
+	my ($ctx) = @_;
+	my $url_prefix = $ctx->{ibx}->base_url($ctx->{env});
 	my $preamble = <<"";
 /*
  * Firefox users: this goes in \$PROFILE_FOLDER/chrome/userContent.css
@@ -87,15 +87,16 @@ sub sample ($$) {
  */
 \@-moz-document url-prefix($url_prefix) { /* moz-only */
 
-	$preamble . CSS() . "\n} /* moz-only */\n";
+	$preamble . CSS . "\n} /* moz-only */\n";
 }
 
 # Auto-update this file based on the contents of a CSS file:
 # usage: perl -I lib __FILE__ contrib/css/216dark.css
 # (See Makefile.PL)
 if (scalar(@ARGV) == 1 && -r __FILE__) {
-	open my $ro, '<', $ARGV[0] or die $!;
-	my $css = do { local $/; <$ro> } or die $!;
+	require PublicInbox::IO;
+	open my $ro, '<', $ARGV[0];
+	my $css = PublicInbox::IO::read_all($ro);
 
 	# indent one level:
 	$css =~ s/^([ \t]*\S)/\t$1/smg;
@@ -104,13 +105,13 @@ if (scalar(@ARGV) == 1 && -r __FILE__) {
 	$css =~ s/;/ !important;/sg;
 	$css =~ s/(\w) \}/$1 !important }/msg;
 
-	open my $rw, '+<', __FILE__ or die $!;
-	my $out = do { local $/; <$rw> } or die $!;
+	open my $rw, '+<', __FILE__;
+	my $out = PublicInbox::IO::read_all($rw);
 	$css =~ s/; /;\n\t\t/g;
 	$out =~ s/^sub CSS.*^_\n\}/sub CSS () {\n\t<<'_'\n${css}_\n}/sm;
 	seek $rw, 0, 0;
-	print $rw $out or die $!;
-	close $rw or die $!;
+	print $rw $out;
+	close $rw;
 }
 
 1;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 1bc2966d..32d0410c 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -681,8 +681,7 @@ sub get_css ($$$) {
 		stylesheets_prepare($self, defined($inbox) ? '' : '+/');
 	my $css = $css_map->{$key};
 	if (!defined($css) && defined($inbox) && $key eq 'userContent') {
-		my $env = $ctx->{env};
-		$css = PublicInbox::UserContent::sample($ctx->{ibx}, $env);
+		$css = PublicInbox::UserContent::sample($ctx);
 	}
 	defined $css or return r404();
 	my $h = [ 'Content-Length', length($css), 'Content-Type', 'text/css' ];
diff --git a/lib/PublicInbox/WwwText.pm b/lib/PublicInbox/WwwText.pm
index 20b22136..79fe46aa 100644
--- a/lib/PublicInbox/WwwText.pm
+++ b/lib/PublicInbox/WwwText.pm
@@ -77,9 +77,7 @@ sub get_text {
 
 sub _colors_help ($$) {
 	my ($ctx, $txt) = @_;
-	my $ibx = $ctx->{ibx};
-	my $env = $ctx->{env};
-	my $base_url = $ibx->base_url($env);
+	my $base_url = $ctx->{ibx}->base_url($ctx->{env});
 	$$txt .= "color customization for $base_url\n";
 	$$txt .= <<EOF;
 
@@ -96,7 +94,7 @@ CSS sample
 ----------
 ```css
 EOF
-	$$txt .= PublicInbox::UserContent::sample($ibx, $env) . "```\n";
+	$$txt .= PublicInbox::UserContent::sample($ctx) . "```\n";
 }
 
 # git-config section names are quoted in the config file, so escape them

[PATCH 2/5] www: don't reread CSS files

[Eric Wong]

With preloading, we usually read our CSS files in quick
succession so they are unlikely to change in between loads.
Thus we can save some syscalls and memory (assuming newer
Perl with CoW scalars).

Since our normal Perl code is only loaded once and ignores
changes on the FS after startup, we'll treat our CSS the
same way and assume they don't change after startup.
---
 lib/PublicInbox/WWW.pm | 48 ++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 32d0410c..6f7c30b9 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -565,6 +565,20 @@ sub get_attach {
 # <pre>-formatted anyways.
 our $STYLE = 'pre{white-space:pre-wrap}*{font-size:100%;font-family:monospace}';
 
+sub _read_css ($$$) {
+	my ($fh, $mini, $fn) = @_;
+	my $ctime = 0;
+	my $local = PublicInbox::IO::read_all $fh; # sets _
+	if ($local =~ /\S/) {
+		$ctime = sprintf('%x',(stat(_))[10]);
+		$local = $mini->($local);
+	}
+	# do not let BOFHs override userContent.css:
+	return ($local, $ctime) if $local !~ /!\s*important\b/i;
+	warn "W: ignoring $fn since it uses `!important'\n";
+	();
+}
+
 sub stylesheets_prepare ($$) {
 	my ($self, $upfx) = @_;
 	my $mini = eval {
@@ -575,7 +589,7 @@ sub stylesheets_prepare ($$) {
 		sub { CSS::Minifier::XS::minify($_[0]) };
 	} || sub { $_[0] };
 
-	my $css_map = {};
+	my $css_map = $self->{-css_map} //= {};
 	my $stylesheets = $self->{pi_cfg}->{css} || [];
 	my $links = [];
 	my $inline_ok = 1;
@@ -598,24 +612,17 @@ sub stylesheets_prepare ($$) {
 				warn "ignoring $fn, non-ASCII word character\n";
 				next;
 			}
-			open(my $fh, '<', $fn) or do {
+			my ($local, $ctime);
+			if (my $rec = $css_map->{$key}) { # already loaded
+				($local, $ctime) = @$rec;
+			} elsif (open(my $fh, '<', $fn)) {
+				($local, $ctime) = _read_css $fh, $mini, $fn;
+				$css_map->{$key} = [ $local, $ctime ];
+			} else {
 				warn "failed to open $fn: $!\n";
 				next;
-			};
-			my $ctime = 0;
-			my $local = PublicInbox::IO::read_all $fh; # sets _
-			if ($local =~ /\S/) {
-				$ctime = sprintf('%x',(stat(_))[10]);
-				$local = $mini->($local);
-			}
-
-			# do not let BOFHs override userContent.css:
-			if ($local =~ /!\s*important\b/i) {
-				warn "ignoring $fn since it uses `!important'\n";
-				next;
 			}
 
-			$css_map->{$key} = $local;
 			$attr->{href} = "$upfx$key.css?$ctime";
 			if (defined($attr->{title})) {
 				$inline_ok = 0;
@@ -654,7 +661,7 @@ sub stylesheets_prepare ($$) {
 	} else {
 		$self->{-style_inline} = $buf;
 	}
-	$self->{-css_map} = $css_map;
+	$css_map;
 }
 
 # returns an HTML fragment with <style> or <link> tags in them
@@ -679,11 +686,12 @@ sub get_css ($$$) {
 	my $self = $ctx->{www};
 	my $css_map = $self->{-css_map} ||
 		stylesheets_prepare($self, defined($inbox) ? '' : '+/');
-	my $css = $css_map->{$key};
-	if (!defined($css) && defined($inbox) && $key eq 'userContent') {
-		$css = PublicInbox::UserContent::sample($ctx);
+	my $rec = $css_map->{$key};
+	if (!defined($rec) && defined($inbox) && $key eq 'userContent') {
+		$rec = [ PublicInbox::UserContent::sample($ctx) ];
 	}
-	defined $css or return r404();
+	$rec // return r404();
+	my ($css, undef) = @$rec; # TODO: Last-Modified
 	my $h = [ 'Content-Length', length($css), 'Content-Type', 'text/css' ];
 	PublicInbox::GitHTTPBackend::cache_one_year($h);
 	[ 200, $h, [ $css ] ];

[PATCH 3/5] www: load CSS files in same dir if @import is in use

[Eric Wong]

In case user-supplied CSS uses @import statements, load any
other unspecified CSS files in that directory to ensure they can
be accessed by clients.
---
 lib/PublicInbox/WWW.pm | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 6f7c30b9..65f95eb8 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -13,6 +13,7 @@
 package PublicInbox::WWW;
 use strict;
 use v5.10.1;
+use autodie qw(chdir opendir);
 use PublicInbox::Config;
 use PublicInbox::Git;
 use PublicInbox::Hval;
@@ -593,6 +594,7 @@ sub stylesheets_prepare ($$) {
 	my $stylesheets = $self->{pi_cfg}->{css} || [];
 	my $links = [];
 	my $inline_ok = 1;
+	my (%css_dir, @css_dir);
 
 	foreach my $s (@$stylesheets) {
 		my $attr = {};
@@ -606,8 +608,9 @@ sub stylesheets_prepare ($$) {
 		if (defined $attr->{href}) {
 			$inline_ok = 0;
 		} else {
-			my $fn = $_;
-			my ($key) = (m!([^/]+?)(?:\.css)?\z!i);
+			my ($fn, $dir, $key);
+			$fn = $_;
+			($dir, $key) = (m!\A(?:(.+?)/)?([^/]+?)(?:\.css)?\z!i);
 			if ($key !~ /\A[a-zA-Z0-9_\-\.]+\z/) {
 				warn "ignoring $fn, non-ASCII word character\n";
 				next;
@@ -617,6 +620,8 @@ sub stylesheets_prepare ($$) {
 				($local, $ctime) = @$rec;
 			} elsif (open(my $fh, '<', $fn)) {
 				($local, $ctime) = _read_css $fh, $mini, $fn;
+				$local =~ /\@import\b/ && !$css_dir{$dir}++ and
+					push @css_dir, $dir;
 				$css_map->{$key} = [ $local, $ctime ];
 			} else {
 				warn "failed to open $fn: $!\n";
@@ -661,6 +666,31 @@ sub stylesheets_prepare ($$) {
 	} else {
 		$self->{-style_inline} = $buf;
 	}
+	# load potentially imported CSS files in known CSS directories
+	if (@css_dir && !$self->{-css_dir}) {
+		opendir my $cwddh, '.';
+		for my $d (@css_dir) {
+			CORE::opendir my $dh, $d or do {
+				warn "W: opendir($d): $!";
+				next;
+			};
+			chdir $dh;
+			my @css = grep /\.css\z/i, readdir $dh;
+			for my $fn (@css) {
+				my ($key) = ($fn =~ m!([^/]+?)(?:\.css)?\z!i);
+				next if $css_map->{$key};
+				-f $fn or next;
+				# no warning for autoloaded CSS
+				open my $fh, '<', $fn or next;
+				-T $fh or next;
+				my ($local, $ctime) =
+						_read_css $fh, $mini, "$d/$fn";
+				$css_map->{$key} = [ $local, $ctime ];
+			}
+			chdir $cwddh;
+		}
+		$self->{-css_dir} = \@css_dir;
+	}
 	$css_map;
 }
 

[PATCH 4/5] www: allow specifying CSS @import or <link> tags

[Eric Wong]

Apparently, some browsers (or settings/extensions) will not
honor certain media= attributes in HTML <link> tags.  So as a
workaround, users may force the use @import statements inside the
<style> tag before the normal monospace CSS $STYLE using the new
`load' directive.

I've only lightly tested due to swap thrashing on my system, but it
appears to work on a new Firefox profile w/ ui.systemUsesDarkTheme=1.
I couldn't get @import nor <link> working on an existing profile,
likely due to some other settings in the config.

I initially wanted to use @import by default, but the ordering
constraint prevents user-specified CSS from overriding the
default $STYLE we normally inject first.

This also ensures @import use inside a specified CSS file forces
the file to be imported first, before the default $STYLE is
inlined.

In the future, `load' may support `last' as a comma-delimited
directive to load CSS at the bottom of the page (before the
`</html>' tag).

Reported-by: Robin H. Johson <robbat2@gentoo.org>
Link: https://public-inbox.org/meta/robbat2-20240901T045635-169490258Z@orbis-terrarum.net/
---
 Documentation/public-inbox-config.pod | 18 +++++++----
 lib/PublicInbox/WWW.pm                | 43 ++++++++++++++++++++++-----
 2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/Documentation/public-inbox-config.pod b/Documentation/public-inbox-config.pod
index 50746b21..e968bc49 100644
--- a/Documentation/public-inbox-config.pod
+++ b/Documentation/public-inbox-config.pod
@@ -255,13 +255,21 @@ Default: :all
 
 =item publicinbox.css
 
-The local path name of a CSS file for the PSGI web interface.
-May contain the attributes "media", "title" and "href" which match
-the associated attributes of the HTML <style> tag.
-"href" may be specified to point to the URL of a remote CSS file
+The local path name of a CSS file for the PSGI web interface.  May
+contain the attributes C<media>, C<title> and C<href> which match the
+associated attributes of the HTML C<E<lt>styleE<gt>> tag.
+
+As of public-inbox 2.0+, C<load> may be C<link> or C<import>;
+C<link> forces the use of a C<E<lt>linkE<gt>> tag and disable inlining
+CSS into the C<E<lt>styleE<gt>> tag while C<import> puts CSS C<@import>
+statements into the C<E<lt>styleE<gt>> tag.  C<import> appears necessary
+for some browser and C<media> query combinations.
+C<href> may be specified to point to the URL of a remote CSS file
 and the path may be "/dev/null" or any empty file.
+
 Multiple files may be specified and will be included in the
-order specified.
+order specified, but files specified via C<load=import> are
+always handled first.
 
 =item publicinboxImport.dropUniqueUnsubscribe
 
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 65f95eb8..60c437a5 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -594,17 +594,28 @@ sub stylesheets_prepare ($$) {
 	my $stylesheets = $self->{pi_cfg}->{css} || [];
 	my $links = [];
 	my $inline_ok = 1;
-	my (%css_dir, @css_dir);
+	my (%css_dir, @css_dir, $import);
 
 	foreach my $s (@$stylesheets) {
 		my $attr = {};
 		local $_ = $s;
-		foreach my $k (qw(media title href)) {
+		foreach my $k (qw(media title href load)) {
 			if (s/\s*$k='([^']+)'// || s/\s*$k=(\S+)//) {
 				$attr->{$k} = $1;
 			}
 		}
-
+		# we may support `last' (to load CSS last at </html>)
+		# or other load directives in the future...
+		for my $l (split /,/, delete $attr->{load} // '') {
+			if ($l eq 'import') {
+				$inline_ok = 0;
+				$import = $attr->{-do_import} = 1;
+			} elsif ($l eq 'link') {
+				$inline_ok = 0;
+			} else {
+				warn "W: load=$l not recognized (ignored)\n";
+			}
+		}
 		if (defined $attr->{href}) {
 			$inline_ok = 0;
 		} else {
@@ -620,8 +631,10 @@ sub stylesheets_prepare ($$) {
 				($local, $ctime) = @$rec;
 			} elsif (open(my $fh, '<', $fn)) {
 				($local, $ctime) = _read_css $fh, $mini, $fn;
-				$local =~ /\@import\b/ && !$css_dir{$dir}++ and
-					push @css_dir, $dir;
+				if ($local =~ /\@import\b/) {
+					$import = $attr->{-do_import} = 1
+					push @css_dir, $dir if !$css_dir{$dir}++
+				}
 				$css_map->{$key} = [ $local, $ctime ];
 			} else {
 				warn "failed to open $fn: $!\n";
@@ -629,7 +642,7 @@ sub stylesheets_prepare ($$) {
 			}
 
 			$attr->{href} = "$upfx$key.css?$ctime";
-			if (defined($attr->{title})) {
+			if (defined($attr->{title})) { # browser-selectable
 				$inline_ok = 0;
 			} elsif (($attr->{media}||'screen') eq 'screen') {
 				$attr->{-inline} = $local;
@@ -637,8 +650,22 @@ sub stylesheets_prepare ($$) {
 		}
 		push @$links, $attr;
 	}
-
-	my $buf = "<style>$STYLE";
+	my $buf = '<style>';
+	if ($import) {
+		my @links;
+		for my $attr (@$links) {
+			if (delete $attr->{-do_import}) {
+				$buf .= '@import url("'.$attr->{href}.'") '.
+					$attr->{media}.';';
+			} else {
+				push @links, $attr;
+			}
+		}
+		$links = \@links;
+	}
+	# can't have $STYLE before @import(?)
+	# <https://developer.mozilla.org/en-US/docs/Web/CSS/@import>
+	$buf .= $STYLE;
 	if ($inline_ok) {
 		my @ext; # for media=print and whatnot
 		foreach my $attr (@$links) {

[PATCH 5/5] www: use mtime as CSS cache-buster instead of ctime

[Eric Wong]

mtime can be synchronized across multiple machines via package
managers, tarballs, rsync deploys, or tools like
`git-set-file-times'.  This synchronization increases cache hit
rates for browsers hitting multi-host public-inbox instances
loadbalancing behind a single $hostname:$port identity.

While mtime may be less correct, it's unusual that anyone would
want to intentionally alter or preserve mtime after a file is
changed on the FS.
---
 lib/PublicInbox/UserContent.pm |  5 +++--
 lib/PublicInbox/WWW.pm         | 22 +++++++++++-----------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/lib/PublicInbox/UserContent.pm b/lib/PublicInbox/UserContent.pm
index 9dacfa0b..1ad8eba7 100644
--- a/lib/PublicInbox/UserContent.pm
+++ b/lib/PublicInbox/UserContent.pm
@@ -74,7 +74,7 @@ _
 sub sample ($) {
 	my ($ctx) = @_;
 	my $url_prefix = $ctx->{ibx}->base_url($ctx->{env});
-	my $preamble = <<"";
+	my $css = <<"";
 /*
  * Firefox users: this goes in \$PROFILE_FOLDER/chrome/userContent.css
  * where \$PROFILE_FOLDER is platform-specific
@@ -87,7 +87,8 @@ sub sample ($) {
  */
 \@-moz-document url-prefix($url_prefix) { /* moz-only */
 
-	$preamble . CSS . "\n} /* moz-only */\n";
+	$css .= CSS . "\n} /* moz-only */\n";
+	wantarray ? ($css, (stat(__FILE__))[9]) : $css;
 }
 
 # Auto-update this file based on the contents of a CSS file:
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 60c437a5..06889861 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -568,14 +568,14 @@ our $STYLE = 'pre{white-space:pre-wrap}*{font-size:100%;font-family:monospace}';
 
 sub _read_css ($$$) {
 	my ($fh, $mini, $fn) = @_;
-	my $ctime = 0;
+	my $mtime = 0;
 	my $local = PublicInbox::IO::read_all $fh; # sets _
 	if ($local =~ /\S/) {
-		$ctime = sprintf('%x',(stat(_))[10]);
+		$mtime = sprintf('%x',(stat(_))[9]);
 		$local = $mini->($local);
 	}
 	# do not let BOFHs override userContent.css:
-	return ($local, $ctime) if $local !~ /!\s*important\b/i;
+	return ($local, $mtime) if $local !~ /!\s*important\b/i;
 	warn "W: ignoring $fn since it uses `!important'\n";
 	();
 }
@@ -626,22 +626,22 @@ sub stylesheets_prepare ($$) {
 				warn "ignoring $fn, non-ASCII word character\n";
 				next;
 			}
-			my ($local, $ctime);
+			my ($local, $mtime);
 			if (my $rec = $css_map->{$key}) { # already loaded
-				($local, $ctime) = @$rec;
+				($local, $mtime) = @$rec;
 			} elsif (open(my $fh, '<', $fn)) {
-				($local, $ctime) = _read_css $fh, $mini, $fn;
+				($local, $mtime) = _read_css $fh, $mini, $fn;
 				if ($local =~ /\@import\b/) {
 					$import = $attr->{-do_import} = 1
 					push @css_dir, $dir if !$css_dir{$dir}++
 				}
-				$css_map->{$key} = [ $local, $ctime ];
+				$css_map->{$key} = [ $local, $mtime ];
 			} else {
 				warn "failed to open $fn: $!\n";
 				next;
 			}
 
-			$attr->{href} = "$upfx$key.css?$ctime";
+			$attr->{href} = "$upfx$key.css?$mtime";
 			if (defined($attr->{title})) { # browser-selectable
 				$inline_ok = 0;
 			} elsif (($attr->{media}||'screen') eq 'screen') {
@@ -710,9 +710,9 @@ sub stylesheets_prepare ($$) {
 				# no warning for autoloaded CSS
 				open my $fh, '<', $fn or next;
 				-T $fh or next;
-				my ($local, $ctime) =
+				my ($local, $mtime) =
 						_read_css $fh, $mini, "$d/$fn";
-				$css_map->{$key} = [ $local, $ctime ];
+				$css_map->{$key} = [ $local, $mtime ];
 			}
 			chdir $cwddh;
 		}
@@ -748,7 +748,7 @@ sub get_css ($$$) {
 		$rec = [ PublicInbox::UserContent::sample($ctx) ];
 	}
 	$rec // return r404();
-	my ($css, undef) = @$rec; # TODO: Last-Modified
+	my ($css, undef) = @$rec; # TODO: Last-Modified + If-Modified-Since
 	my $h = [ 'Content-Length', length($css), 'Content-Type', 'text/css' ];
 	PublicInbox::GitHTTPBackend::cache_one_year($h);
 	[ 200, $h, [ $css ] ];

Re: [PATCH 4/5] www: allow specifying CSS @import or <link> tags

[Štěpán Němec]

On Thu, 26 Sep 2024 00:55:04 +0000
Eric Wong wrote:

[...]

> diff --git a/Documentation/public-inbox-config.pod b/Documentation/public-inbox-config.pod
> index 50746b21..e968bc49 100644
> --- a/Documentation/public-inbox-config.pod
> +++ b/Documentation/public-inbox-config.pod
> @@ -255,13 +255,21 @@ Default: :all
>  
>  =item publicinbox.css
>  
> -The local path name of a CSS file for the PSGI web interface.
> -May contain the attributes "media", "title" and "href" which match
> -the associated attributes of the HTML <style> tag.
> -"href" may be specified to point to the URL of a remote CSS file
> +The local path name of a CSS file for the PSGI web interface.  May
> +contain the attributes C<media>, C<title> and C<href> which match the
> +associated attributes of the HTML C<E<lt>styleE<gt>> tag.
> +
> +As of public-inbox 2.0+, C<load> may be C<link> or C<import>;
> +C<link> forces the use of a C<E<lt>linkE<gt>> tag and disable inlining
                                                         ^^^^^^^
disables

I admit that reading this I had no idea what this was
talking about at first, as in, local path name "may contain
attributes"?  Looking at the code, it seems somewhat in the
style of gitattributes(5), plus you handle single-quoted
string values (i.e., embedded spaces), unlike git (looking
at attr.c:parse_attr).

I wonder if a bit of hand-holding (AKA documentation)
could/should be provided here?

Perhaps, if I understand the behavior correctly:

  The local path name of a CSS file for the PSGI web interface.
  Space-separated attributes (C<key=val> or C<key='val with spaces'>)
  may be specified after the path on the same line; C<media>, C<title>
  and C<href> are currently supported, and match the associated attributes
  of the HTML C<E<lt>styleE<gt>> tag.
  
  public-inbox 2.0+ also supports a C<load> attribute with values
  C<link> or C<import>; C<link> forces the use of a C<E<lt>linkE<gt>> tag
  and disables inlining [...]

> +CSS into the C<E<lt>styleE<gt>> tag while C<import> puts CSS C<@import>
> +statements into the C<E<lt>styleE<gt>> tag.  C<import> appears necessary
> +for some browser and C<media> query combinations.
> +C<href> may be specified to point to the URL of a remote CSS file
>  and the path may be "/dev/null" or any empty file.
> +
>  Multiple files may be specified and will be included in the
> -order specified.
> +order specified, but files specified via C<load=import> are
> +always handled first.

[...]

-- 
Štěpán

[PATCH v2] www: allow specifying CSS @import or <link> tags

[Eric Wong]

Štěpán Němec <stepnem@smrk.net> wrote:
> I admit that reading this I had no idea what this was
> talking about at first, as in, local path name "may contain
> attributes"?  Looking at the code, it seems somewhat in the
> style of gitattributes(5), plus you handle single-quoted
> string values (i.e., embedded spaces), unlike git (looking
> at attr.c:parse_attr).

Yup.  I basically wanted to map HTML attributes as-is, but
`load' is a special case.

> I wonder if a bit of hand-holding (AKA documentation)
> could/should be provided here?
> 
> Perhaps, if I understand the behavior correctly:
> 
>   The local path name of a CSS file for the PSGI web interface.
>   Space-separated attributes (C<key=val> or C<key='val with spaces'>)
>   may be specified after the path on the same line; C<media>, C<title>
>   and C<href> are currently supported, and match the associated attributes
>   of the HTML C<E<lt>styleE<gt>> tag.
>   
>   public-inbox 2.0+ also supports a C<load> attribute with values
>   C<link> or C<import>; C<link> forces the use of a C<E<lt>linkE<gt>> tag
>   and disables inlining [...]

Much better, thanks.  I used your text for v2.

------8<------
Subject: [PATCH] www: allow specifying CSS @import or <link> tags

Apparently, some browsers (or settings/extensions) will not
honor certain media= attributes in HTML <link> tags.  So as a
workaround, users may force the use @import statements inside the
<style> tag before the normal monospace CSS $STYLE using the new
`load' directive.

I've only lightly tested due to swap thrashing on my system, but it
appears to work on a new Firefox profile w/ ui.systemUsesDarkTheme=1.
I couldn't get @import nor <link> working on an existing profile,
likely due to some other settings in the config.

I initially wanted to use @import by default, but the ordering
constraint prevents user-specified CSS from overriding the
default $STYLE we normally inject first.

This also ensures @import use inside a specified CSS file forces
the file to be imported first, before the default $STYLE is
inlined.

In the future, `load' may support `last' as a comma-delimited
directive to load CSS at the bottom of the page (before the
`</html>' tag).

Reported-by: Robin H. Johson <robbat2@gentoo.org>
Link: https://public-inbox.org/meta/robbat2-20240901T045635-169490258Z@orbis-terrarum.net/
Helped-by: Štěpán Němec <stepnem@smrk.net>
---
 Documentation/public-inbox-config.pod | 30 ++++++++++++++++---
 lib/PublicInbox/WWW.pm                | 43 ++++++++++++++++++++++-----
 2 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/Documentation/public-inbox-config.pod b/Documentation/public-inbox-config.pod
index 50746b21..667f626f 100644
--- a/Documentation/public-inbox-config.pod
+++ b/Documentation/public-inbox-config.pod
@@ -256,12 +256,34 @@ Default: :all
 =item publicinbox.css
 
 The local path name of a CSS file for the PSGI web interface.
-May contain the attributes "media", "title" and "href" which match
-the associated attributes of the HTML <style> tag.
-"href" may be specified to point to the URL of a remote CSS file
+Space-separated attributes (C<key=val> or C<key='val with spaces'>)
+may be specified after the path on the same line; C<media>, C<title>
+and C<href> are currently supported, and match the associated attributes
+of the HTML C<E<lt>styleE<gt>> tag.
+
+C<href> may be specified to point to the URL of a remote CSS file
 and the path may be "/dev/null" or any empty file.
+
+public-inbox 2.0+ also supports a C<load> attribute with values
+C<link> or C<import>; C<link> forces the use of a C<E<lt>linkE<gt>> tag
+and disables inlining CSS into the C<E<lt>styleE<gt>> tag while
+C<import> puts CSS C<@import> statements into the C<E<lt>styleE<gt>> tag.
+C<import> appears necessary for some browser and C<media> query combinations.
+
 Multiple files may be specified and will be included in the
-order specified.
+order specified, but files specified via C<load=import> are
+always handled first.
+
+Example:
+
+	[publicinbox]
+		css = /path/to/216dark.css title=216dark \
+		  media='screen,(prefers-color-scheme:dark)'
+
+See the L<contrib/css
+directory of the public-inbox
+source|https://80x24.org/public-inbox.git/tree/contrib/css>
+for more examples.
 
 =item publicinboxImport.dropUniqueUnsubscribe
 
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 65f95eb8..60c437a5 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -594,17 +594,28 @@ sub stylesheets_prepare ($$) {
 	my $stylesheets = $self->{pi_cfg}->{css} || [];
 	my $links = [];
 	my $inline_ok = 1;
-	my (%css_dir, @css_dir);
+	my (%css_dir, @css_dir, $import);
 
 	foreach my $s (@$stylesheets) {
 		my $attr = {};
 		local $_ = $s;
-		foreach my $k (qw(media title href)) {
+		foreach my $k (qw(media title href load)) {
 			if (s/\s*$k='([^']+)'// || s/\s*$k=(\S+)//) {
 				$attr->{$k} = $1;
 			}
 		}
-
+		# we may support `last' (to load CSS last at </html>)
+		# or other load directives in the future...
+		for my $l (split /,/, delete $attr->{load} // '') {
+			if ($l eq 'import') {
+				$inline_ok = 0;
+				$import = $attr->{-do_import} = 1;
+			} elsif ($l eq 'link') {
+				$inline_ok = 0;
+			} else {
+				warn "W: load=$l not recognized (ignored)\n";
+			}
+		}
 		if (defined $attr->{href}) {
 			$inline_ok = 0;
 		} else {
@@ -620,8 +631,10 @@ sub stylesheets_prepare ($$) {
 				($local, $ctime) = @$rec;
 			} elsif (open(my $fh, '<', $fn)) {
 				($local, $ctime) = _read_css $fh, $mini, $fn;
-				$local =~ /\@import\b/ && !$css_dir{$dir}++ and
-					push @css_dir, $dir;
+				if ($local =~ /\@import\b/) {
+					$import = $attr->{-do_import} = 1
+					push @css_dir, $dir if !$css_dir{$dir}++
+				}
 				$css_map->{$key} = [ $local, $ctime ];
 			} else {
 				warn "failed to open $fn: $!\n";
@@ -629,7 +642,7 @@ sub stylesheets_prepare ($$) {
 			}
 
 			$attr->{href} = "$upfx$key.css?$ctime";
-			if (defined($attr->{title})) {
+			if (defined($attr->{title})) { # browser-selectable
 				$inline_ok = 0;
 			} elsif (($attr->{media}||'screen') eq 'screen') {
 				$attr->{-inline} = $local;
@@ -637,8 +650,22 @@ sub stylesheets_prepare ($$) {
 		}
 		push @$links, $attr;
 	}
-
-	my $buf = "<style>$STYLE";
+	my $buf = '<style>';
+	if ($import) {
+		my @links;
+		for my $attr (@$links) {
+			if (delete $attr->{-do_import}) {
+				$buf .= '@import url("'.$attr->{href}.'") '.
+					$attr->{media}.';';
+			} else {
+				push @links, $attr;
+			}
+		}
+		$links = \@links;
+	}
+	# can't have $STYLE before @import(?)
+	# <https://developer.mozilla.org/en-US/docs/Web/CSS/@import>
+	$buf .= $STYLE;
 	if ($inline_ok) {
 		my @ext; # for media=print and whatnot
 		foreach my $attr (@$links) {

Re: [PATCH v2] www: allow specifying CSS @import or <link> tags

[Štěpán Němec]

On Sat, 28 Sep 2024 18:39:35 +0000
Eric Wong wrote:

> diff --git a/Documentation/public-inbox-config.pod b/Documentation/public-inbox-config.pod
> index 50746b21..667f626f 100644
> --- a/Documentation/public-inbox-config.pod
> +++ b/Documentation/public-inbox-config.pod
> @@ -256,12 +256,34 @@ Default: :all
>  =item publicinbox.css
>  
>  The local path name of a CSS file for the PSGI web interface.
> -May contain the attributes "media", "title" and "href" which match
> -the associated attributes of the HTML <style> tag.
> -"href" may be specified to point to the URL of a remote CSS file
> +Space-separated attributes (C<key=val> or C<key='val with spaces'>)
> +may be specified after the path on the same line; C<media>, C<title>
> +and C<href> are currently supported, and match the associated attributes
> +of the HTML C<E<lt>styleE<gt>> tag.
> +
> +C<href> may be specified to point to the URL of a remote CSS file
>  and the path may be "/dev/null" or any empty file.

This is indeed a better place for that sentence; the
displacement in v1 gave me a pause, but I didn't really
connect the dots...

> +public-inbox 2.0+ also supports a C<load> attribute with values
> +C<link> or C<import>; C<link> forces the use of a C<E<lt>linkE<gt>> tag
> +and disables inlining CSS into the C<E<lt>styleE<gt>> tag while
> +C<import> puts CSS C<@import> statements into the C<E<lt>styleE<gt>> tag.
> +C<import> appears necessary for some browser and C<media> query combinations.
> +
>  Multiple files may be specified and will be included in the
> -order specified.
> +order specified, but files specified via C<load=import> are
> +always handled first.

I guess saying "order given" or "given order" instead would
read better (avoid triple "specified" in a single sentence),
but that's just nit picking.

> +
> +Example:
> +
> +	[publicinbox]
> +		css = /path/to/216dark.css title=216dark \
> +		  media='screen,(prefers-color-scheme:dark)'
> +
> +See the L<contrib/css
> +directory of the public-inbox
> +source|https://80x24.org/public-inbox.git/tree/contrib/css>
> +for more examples.

And an example is helpful (and I'd missed contrib/css, too).

Thanks,

  Štěpán

Re: [PATCH v2] www: allow specifying CSS @import or <link> tags

[Eric Wong]

Štěpán Němec <stepnem@smrk.net> wrote:
> On Sat, 28 Sep 2024 18:39:35 +0000
> Eric Wong wrote:
> >  Multiple files may be specified and will be included in the
> > -order specified.
> > +order specified, but files specified via C<load=import> are
> > +always handled first.
> 
> I guess saying "order given" or "given order" instead would
> read better (avoid triple "specified" in a single sentence),
> but that's just nit picking.

I'll squash this in before pushing:

diff --git a/Documentation/public-inbox-config.pod b/Documentation/public-inbox-config.pod
index 667f626f..6c4d633a 100644
--- a/Documentation/public-inbox-config.pod
+++ b/Documentation/public-inbox-config.pod
@@ -271,7 +271,7 @@ C<import> puts CSS C<@import> statements into the C<E<lt>styleE<gt>> tag.
 C<import> appears necessary for some browser and C<media> query combinations.
 
 Multiple files may be specified and will be included in the
-order specified, but files specified via C<load=import> are
+given order, but files specified via C<load=import> are
 always handled first.
 
 Example:

Re: [PATCH v2] www: allow specifying CSS @import or <link> tags

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> I'll squash this in before pushing:

And this, totally forgot to test the final patch :x

diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 60c437a5..987d644e 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -632,7 +632,7 @@ sub stylesheets_prepare ($$) {
 			} elsif (open(my $fh, '<', $fn)) {
 				($local, $ctime) = _read_css $fh, $mini, $fn;
 				if ($local =~ /\@import\b/) {
-					$import = $attr->{-do_import} = 1
+					$import = $attr->{-do_import} = 1;
 					push @css_dir, $dir if !$css_dir{$dir}++
 				}
 				$css_map->{$key} = [ $local, $ctime ];