* [PATCH 1/5] user_content: simplify internal API and use v5.12
2024-09-26 0:55 ` [PATCH 0/5] css improvements (hopefully) Eric Wong
@ 2024-09-26 0:55 ` Eric Wong
2024-09-26 0:55 ` [PATCH 2/5] www: don't reread CSS files Eric Wong
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-09-26 0:55 UTC (permalink / raw)
To: meta; +Cc: Robin H. Johnson
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/5] www: don't reread CSS files
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 ` Eric Wong
2024-09-26 0:55 ` [PATCH 3/5] www: load CSS files in same dir if @import is in use Eric Wong
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-09-26 0:55 UTC (permalink / raw)
To: meta; +Cc: Robin H. Johnson
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 ] ];
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/5] www: load CSS files in same dir if @import is in use
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 ` Eric Wong
2024-09-26 0:55 ` [PATCH 4/5] www: allow specifying CSS @import or <link> tags Eric Wong
2024-09-26 0:55 ` [PATCH 5/5] www: use mtime as CSS cache-buster instead of ctime Eric Wong
4 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-09-26 0:55 UTC (permalink / raw)
To: meta; +Cc: Robin H. Johnson
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;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/5] www: allow specifying CSS @import or <link> tags
2024-09-26 0:55 ` [PATCH 0/5] css improvements (hopefully) Eric Wong
` (2 preceding siblings ...)
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 ` Eric Wong
2024-09-26 18:34 ` Štěpán Němec
2024-09-26 0:55 ` [PATCH 5/5] www: use mtime as CSS cache-buster instead of ctime Eric Wong
4 siblings, 1 reply; 13+ messages in thread
From: Eric Wong @ 2024-09-26 0:55 UTC (permalink / raw)
To: meta; +Cc: Robin H. Johnson
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) {
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/5] www: allow specifying CSS @import or <link> tags
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 ` [PATCH v2] " Eric Wong
0 siblings, 1 reply; 13+ messages in thread
From: Štěpán Němec @ 2024-09-26 18:34 UTC (permalink / raw)
To: Eric Wong; +Cc: meta, Robin H. Johnson
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2] www: allow specifying CSS @import or <link> tags
2024-09-26 18:34 ` Štěpán Němec
@ 2024-09-28 18:39 ` Eric Wong
2024-09-28 19:29 ` Štěpán Němec
0 siblings, 1 reply; 13+ messages in thread
From: Eric Wong @ 2024-09-28 18:39 UTC (permalink / raw)
To: Štěpán Němec; +Cc: meta, Robin H. Johnson
Š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) {
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] www: allow specifying CSS @import or <link> tags
2024-09-28 18:39 ` [PATCH v2] " Eric Wong
@ 2024-09-28 19:29 ` Štěpán Němec
2024-09-28 20:42 ` Eric Wong
0 siblings, 1 reply; 13+ messages in thread
From: Štěpán Němec @ 2024-09-28 19:29 UTC (permalink / raw)
To: Eric Wong; +Cc: meta, Robin H. Johnson
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] www: allow specifying CSS @import or <link> tags
2024-09-28 19:29 ` Štěpán Němec
@ 2024-09-28 20:42 ` Eric Wong
2024-09-29 0:22 ` Eric Wong
0 siblings, 1 reply; 13+ messages in thread
From: Eric Wong @ 2024-09-28 20:42 UTC (permalink / raw)
To: Štěpán Němec; +Cc: meta, Robin H. Johnson
Š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:
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] www: allow specifying CSS @import or <link> tags
2024-09-28 20:42 ` Eric Wong
@ 2024-09-29 0:22 ` Eric Wong
0 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-09-29 0:22 UTC (permalink / raw)
To: meta; +Cc: Robin H. Johnson, Štěpán Němec
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 ];
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/5] www: use mtime as CSS cache-buster instead of ctime
2024-09-26 0:55 ` [PATCH 0/5] css improvements (hopefully) Eric Wong
` (3 preceding siblings ...)
2024-09-26 0:55 ` [PATCH 4/5] www: allow specifying CSS @import or <link> tags Eric Wong
@ 2024-09-26 0:55 ` Eric Wong
4 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-09-26 0:55 UTC (permalink / raw)
To: meta; +Cc: Robin H. Johnson
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 ] ];
^ permalink raw reply related [flat|nested] 13+ messages in thread