unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: "Štěpán Němec" <stepnem@smrk.net>
Cc: meta@public-inbox.org, "Robin H. Johnson" <robbat2@gentoo.org>
Subject: [PATCH v2] www: allow specifying CSS @import or <link> tags
Date: Sat, 28 Sep 2024 18:39:35 +0000	[thread overview]
Message-ID: <20240928183935.M960376@dcvr> (raw)
In-Reply-To: <20240926183402Z.161922298-stepnem@smrk.net>

Š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) {

  reply	other threads:[~2024-09-28 18:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-01  5:07 CSS behaviors bug report: dark/light switch non-functional Robin H. Johnson
2024-09-18  0:04 ` Eric Wong
2024-09-26  0:55 ` [PATCH 0/5] css improvements (hopefully) Eric Wong
2024-09-26  0:55   ` [PATCH 1/5] user_content: simplify internal API and use v5.12 Eric Wong
2024-09-26  0:55   ` [PATCH 2/5] www: don't reread CSS files Eric Wong
2024-09-26  0:55   ` [PATCH 3/5] www: load CSS files in same dir if @import is in use Eric Wong
2024-09-26  0:55   ` [PATCH 4/5] www: allow specifying CSS @import or <link> tags Eric Wong
2024-09-26 18:34     ` Štěpán Němec
2024-09-28 18:39       ` Eric Wong [this message]
2024-09-28 19:29         ` [PATCH v2] " Štěpán Němec
2024-09-28 20:42           ` Eric Wong
2024-09-29  0:22             ` Eric Wong
2024-09-26  0:55   ` [PATCH 5/5] www: use mtime as CSS cache-buster instead of ctime Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240928183935.M960376@dcvr \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    --cc=robbat2@gentoo.org \
    --cc=stepnem@smrk.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).