It seems like the url_var() and image_url_var() code can be simplified, a lot, both on trunk and on the fileedit-ajaxify branch. I would jump in and try to simplify trunk, but I fear that would make it difficult to merge to fileedit-ajaxify. (Update: I did make some changes to the skin_id() routine, which hopefully you can merge without issue.)

The url_var() is only used to set css_url and a couple of variables associated with images from image_url_var(). It seems like we could unwind the layers of abstraction and set css_url, logo_image_url, and background_image_url TH1 variables using a single general-purpose routine that looks something like this:

    void style_set_th1_var(const char *zVarName, const char *zFormat, ....);

Rather than having a page= query parameter, would it work to have a new base URL for that case? Use "/style.css" for the generic CSS, and then "/perpage-style.css" for those cases that require their own special augmented CSS? The page= query parameter is only used for /perpage-style.css.

Radical simplification of all CSS?

For 2.12, can we purpose to go through and simplify all of the CSS, both the common "default_css.txt" file and any custom bits for particular pages, with the aim of having some kind of systematic naming scheme for classes and identifiers and with dramatically reducing the number of rules? This seems long overdue. A CSS refactor will break custom skins, however, so perhaps this will require a new major version number: Version 3.0 instead of 2.12?

It seems like we could unwind the layers of abstraction and set css_url, logo_image_url, and background_image_url TH1 variables using a single general-purpose routine that looks something like this:

i'm not yet familiar enough with those bits to comment on that.

Rather than having a page= query parameter, would it work to have a new base URL for that case? Use "/style.css" for the generic CSS, and then "/perpage-style.css" for those cases that require their own special augmented CSS?

That would have been my preference, and the only reason i didn't go that route was because of the fallout in skins. While it's no problem to add a second CSS include to the checked-in skins, there are lots of skin customizations out there which would need to add the second CSS line to their header (or reset their skins).

It seems that we might be able to improve cacheability if, instead of using a ?page=X for each page, instead name the secondary file after the current page, e.g. style-fileedit.css. Functionally, though, it's six one way and half a dozen the other.

For 2.12, can we purpose to go through and simplify all of the CSS, both the common "default_css.txt" file and any custom bits for particular pages, with the aim of having some kind of systematic naming scheme for classes and identifiers and with dramatically reducing the number of rules?

i would be completely on board with that, and will volunteer to take the lead unless someone else is itching to do it (in which case i am at their disposal), but...

A CSS refactor will break custom skins, however, so perhaps this will require a new major version number: Version 3.0 instead of 2.12?

That.

However, there's no fundamental harm in duplicating CSS rules under different names, so we have the option of adding a new scheme and retaining the old, stripping it out at the next major release. The cost would be duplicate CSS rules for the duration of the migration.

If the defaultcss parser were enhanced to support CSS's comma-separated selectors, we can even use both name styles with no style rule duplication:

diffcoltxt, f-diff-column-text {
  the style...
}

Something along those lines. If any new selectors require, for whatever reason, modified semantics we can do...

diffcoltxt, f-diff-column-text {
  the shared style...
}
f-diff-column-text {
  cascade the new semantics here
}

However, both of those would require modifying the current defaultcss generator to handle comma-separated selectors.

My preference would be to always emit all default rules and use normal CSS cascading in the skins for customization, instead of the emit-default-if-the-skin-did-not approach which style.css currently uses. The current approach produces more compact output but is semantically in conflict with the idea of how CSS overrides are intended to work, burdens skins with matching the built-in selectors precisely, and requires that defaultcss retain the same selectors for all time for compatibility with old skins. i believe that such a shift in how default CSS is processed could be made without breaking current skins, and will take a closer look at the code/process once i finally crawl out of bed and inhale some coffee.

This is yet another rabbit hole, but a long-overdue one.

(Pardon brevity - one-handed from a tablet in bed. More proof that we are living in The Future.)

That would have been my preference, and the only reason i didn't go that route was because of the fallout in skins. While it's no problem to add a second CSS include to the checked-in skins, there are lots of skin customizations out there which would need to add the second CSS line to their header (or reset their skins).

1. All the skins should be omitting the <head> tag and letting Fossil fill that in for them. If they are not already, perhaps a change like this can encourage them to begin.

2. Even if skins are still building their own <head>, they should be using the $stylesheet_url TH1 variable for the URL for the stylesheet they reference, not some hard-coded value. So if you want to set the $stylesheet_url variable to something other than "/style.css" for the /fileedit page, even the customized skins should still work, no?

All the skins should be omitting the <head> tag and letting Fossil fill that in for them. If they are not already, perhaps a change like this can encourage them to begin.

That's not always possible: sites which add their own CSS files have to output their own head. i do this on at least two repos because:

1. Import 3rd-party syntax highlighter CSS.
2. i maintain skin overrides in an external file, instead of the skin editor: it's easier to work with (emacs css-mode), can be served via /doc/ckout (no skin editing required), and can be easily ported across trees, rather than editing each one's skin-level CSS.

Check the sources for <view-source:https://fossil.wanderinghorse.net/r/cwal/home> for an example.

Skins need to be able to emit their own custom HEAD section, or at least to flexibly amend a generated one.

Even if skins are still building their own <head>, they should be using the $stylesheet_url TH1 variable for the URL for the stylesheet they reference, not some hard-coded value. So if you want to set the $stylesheet_url variable to something other than "/style.css" for the /fileedit page, even the customized skins should still work, no?

Correct, and all builtin skins currently do that except (it was discovered earlier today during fileedit testing) bootstrap, which was fixed in 100c67fa507b44ad but still needs to be cherry-picked into trunk.

All the skins should be omitting the <head> tag and letting Fossil fill that in for them. If they are not already, perhaps a change like this can encourage them to begin.

cough from the core repo's skin: cough

<link rel="stylesheet" href="$baseurl/style.css?default" type="text/css"
            media="screen" />

;)

Related: that media=screen part was removed from the default skin in November because it interferes with printing the site.

Rather than having a page= query parameter, would it work to have a new base URL for that case? Use "/style.css" for the generic CSS, and then "/perpage-style.css" for those cases that require their own special augmented CSS? The page= query parameter is only used for /perpage-style.css.

(Back on the PC...)

Similarly, rather than emit $<stylesheet_url>, we could emit $<stylesheets_links>, which contains the source for all <LINK> tags needs needed by the page. One advantage of this would be that we could conditionally emit CSS specific to individual JS modules, e.g. style for the tabbed widget and the fossil status bar¹, in pages which use those modules. Currently that stuff lives in style.fileedit.css, but much of it will eventually migrate to default_css.txt as other pages adopt some of those features. It would be nice to elide those bits from pages which don't need them (which is most pages).

Maybe getting a bit ahead of myself, but my next step, after merging in your style_id() change to fileedit, is to throw together (in a new branch based off of trunk) my proposal of how the default CSS handling "should" be implemented, and "could" be implemented without (i believe) major fallout in terms of compatibility with current repos. Namely:

1. Unconditionally emit all of defaultcss_txt.
2. Emit page-specific CSS, if any, in which each page may extend/override defaults and provide new styles. (There is none in trunk, so this is currently a no-op.)
3. Emit skin-side CSS, which may cascade or completely override rules for any of (1) or (2).

Noting that whether this is all done via a single central style.css or emitting (up to) 3 separate <LINK> tags is not really important to me. The former is a no-op to integrate with current skins, so it's what i'll prototype with, but the latter has potential caching and separation-of-concerns advantages. That's a detail we can change at any time later on (and i have migration plan for skins which shouldn't be too onerous for clients, but that's a later step.... ooooohhhhh.... we can actually easily support both approaches at the same time... more on that later).

Whereas the current approach is:

1. Emit skin-specific CSS.
2. Loop over default_css.txt rules looking for precise selector matches in (1). If we find no match in (1), emit the default rule, else elide.

That second step unduly burdens both skin developers and skin customizers:

1. CSS selectors in skins must match defaultcss's exactly.
2. defaultcss has to retain selectors forever for skin compatibility.
3. It requires that anyone customizing style customize all aspects which the matching defaulcss selector does, as opposed to being able (in conventional CSS fashion) to override only the aspects which the skinner wants to customize. For example, the following should be possible:

/* in default_css.txt */
foo {
  font-size: 800%;
  color: red;
  background: yellow;
}

/* in the skin's css.txt... */
foo, bar, baz {
   font-size: 200%;
   margin: 17em;
}

/* further down in the user's skin customization... */
foo {
   font-size: 350%;
   color: cyan;
}

With the final browser-computed cascaded rule being:

foo {
  font-size: 350%;
  color: cyan;
  background: yellow;
  margin: 17em;
}

This approach would greatly simplify customization from the defaults, compared to the current all-or-nothing approach in which the user has to explicitly implement all rules of a given selector. The current approach also means that if the default rules are ever extended, clients do not inherit those extensions because the defaults are not emitted if a user emits their own matching selector. e.g. if a default adds a margin value tomorrow, clients who customized that selector will not pick that up. That may arguably be seen as a feature, but IMO it's more arguably a hindrance (in this specific context, anyway).

The current approach's only(?) advantage is that it generates more compact CSS, but (IMO) not by a large (no pun intended) enough margin (also no pun intended) that it's worth the current limitations.

Anyway... code talks and talk walks, so let me go throw that together. (Warren's recent "fossil is a do-ocracy" comment comes to mind.) Back into to the rabbit hole... (Retirement means there's practically no end to the rabbit hole.)

1. ^{^} the code-highlight links are still one of my favorite fossil features.

Anyway... code talks and talk walks, so let me go throw that together.

That was painless. Here's a proof of concept demonstration:

https://fossil-scm.org/fossil/timeline?r=style-css-revamp

To try it out in the core repo, the skin must be edited to patch the old-style style.css declaration to the more modern:

<link rel="stylesheet" href="$stylesheet_url" type="text/css" />

And then visit /admin_log and hover over any links on that page.

Rather than use page=g.zPath i instead opted for the nicer-looking style.css/{{g.zPath}}?id=..., which is functionally the same as passing a parameter, but hides that behind a path-looking construct. That suits me better than the extra URL parameter.

My current thinking is that this can be used as a drop-in replacement for the current mechanism, but i need to go through all of the builtin skins and try it out, looking for unwanted side effects, to be certain.

Thoughts?

My current thinking is that this can be used as a drop-in replacement for the current mechanism, but i need to go through all of the builtin skins and try it out, looking for unwanted side effects, to be certain.

i just ran through all 13 of the builtin skins, trying out all of the pages linked to in the top menu and a random sampling of other pages. With the exception of the timeline colors in the Ardoise skin, everything looks, to my eyes, the same as in trunk.

The CSS size difference is negligible, despite unconditionally emitting all of the default CSS:

• trunk = ~19.5k (4.9k compressed)
• this branch = ~22k (5.09k compressed)

(Edit: these numbers vary by skin - sometimes the uncompressed difference is a matter of 100 bytes, not a few kb. That initial count was also inflated for the demo branch because i inadvertently emitted the skin CSS twice :/.)

With Ardoise timeline colors patched (pending edit: not necessary), this approach would appear to be drop-in compatible with trunk.

With Ardoise timeline colors patched (pending), this approach would appear to be drop-in compatible with trunk.

LOL.

It turns out that's not a problem with this branch: those colors look the same on the trunk version. My initial comparison of colors was off - looking at a repo where the Ardoise timeline colors actually look good :). On the core repo they simply don't :/, both with and without this branch.

So...

This change appears to have zero visible effect on the built-in skins, and no unwanted side-effects are expected on custom skins because they previously had to derive all styles for a given selector, anyway. This gives us, however, a more user-friendly approach to cascading/extending the CSS rules.

Radical simplification of all CSS?

If we're doing that, we should do it with SCSS or a similar competing CSS extension language.

For example, this section of the default CSS becomes this in SCSS:

.title {
    color: #4183C4;
    float:left;
    h1 {
        display:inline;
        &:after {
            content: " / ";
            color: #777;
            font-weight: normal;
        }
    }
}

Aside from expressing nested styles directly in the structure of the declaration, it allows many other things, like variables and calculations on same.

Take the Khaki skin. It has four main colors: background, foreground text, main accent, and secondary accent. The accent colors can be derived from the background color, and all other colors are either reuses of one of these four or can be trivially computed from them. Therefore, instead of all that repeating color stuff, you say something like this instead:

$textcolor: black;
$bgcolor:   hsl(50deg, 26%, 99%);
$maincolor: color.adjust($bgcolor, $saturation: +29%, $lightness: -36%);
$acccolor:  color.adjust($maincolor, $lightness: +12%);

This syntax has many virtues over raw #rrGGbb values:

1. It clearly shows the interrelationship between these three colors: all three have the same hue, and the two accent colors differ only in lightness.

2. My calculations above give colors slightly off the current Khaki skin colors. Why is this a virtue, you ask? Because it corrects an error in the skin's original design, where the current values were clearly picked visually rather than calculated, resulting in small pointless variances. (55° hue in some colors vs. 54° hue in others, for instance.) By recasting all colors this way, you zero out such errors.

3. Once you have one skin converted this way, you can generate a vast range of related skins by just changing a few constants and re-generating the skin. Don't like yellow/brown shades? Okay, change $bgcolor to a blue shade, and now you have "Blue Khaki."

There is an MIT-licensed Sass/SCSS library which we could integrate into Fossil so we can get the benefit of #3 at run time: under Admin → Skins, we can then say "give me Khaki but with a blue $bgcolor" and have Fossil generate a new skin for us.

If for some reason that's unpalatable, there are many standalone SCSS to CSS compilers out there, and we can make one a build-time dependency instead.

I did an SCSS refactor on my main web app a few years back, and it greatly simplified the resulting CSS. I found redundancy after redundancy and recast every one to be in terms of something else I already had. I can now reskin the whole app by adjusting a few constants.

If we're doing that, we should do it with SCSS or a similar competing CSS extension language.

That not only means extra tooling, but almost all of it is based on nodejs (barf). Last summer i looked into that for my website and was appalled at how little C code there is for working with CSS (and ended up having to write my own CSS minifier library).

There is an MIT-licensed Sass/SCSS library which we could integrate into Fossi

"LibSass - Sass compiler written in C++"

While i'm no enemy of C++, i suspect that's going to be a hard sell for integration into this project.

appalled at how little C code there is for working with CSS

There are better languages for working with text file formats than C.

JavaScript wouldn't be my first choice for manipulating generic text file formats, but since Node is based on the same tech stack as Chrome, I'd expect it's got some first-class tools for manipulating CSS in particular.

Sass's initial implementation language (Ruby) would be on my short list. Tcl, Perl, Python...all better choices than C for manipulating CSS text.

While i'm no enemy of C++, i suspect that's going to be a hard sell for integration into this project.

libsass has a C interface, and it has a single-function do-everything interface that may suffice for our purposes. If not, this Tcl Sass module shows that you can do some fairly complex things using only the library's C interface.

It doesn't have to be any more "integrated" than OpenSSL. It's available in most package repos.

There are better languages for working with text file formats than C.

Certainly, but 95%+ of the tools out there for working with CSS are based on node.js, and node is an absolute catastrophe of a tool. Yes, JS is far better for that purpose, but nodejs in a toolchain... Shudder.

It doesn't have to be any more "integrated" than OpenSSL. It's available in most package repos.

OpenSSL is a bit of a special case: it's ubiquitous, as there are few alternatives (possibly none that are anywhere nearly as widespread), so it has a level of portability and availability which far exceeds most 3rd-party packages. Worst case, can be unpacked directly into the fossil tree for building.

Requiring a 3rd-party CSS processor in order to build fossil would almost certainly make it a non-starter for many people who currently get by with ./configure && make.

If style.css is requested with a page=name URL parameter

You don't need the parameter: the code can infer which page is requesting style.css and append the necessary extras based on the HTTP Referer header.

(Yes, it's misspelled; accident of history, can't be changed now.)

...packed into the global CSS, which "just doesn't sit well" with me.

First, keep in mind that this page should be long-term cached, so it's not like you're actually sending /fileedit page CSS with every page hit. The CSS text only goes out once between edits as long as the user visits the page often enough to keep it in the browser cache. Thus, even if you don't use the /fileedit classes, you pay for them rarely. The same goes for other pages some people may rarely or never visit, like the ticket feature, so it's not like you're alone here in demanding space in the global CSS.

Second, I expect the global CSS will be only part of the finished product: I also expect per-skin overrides. Take Khaki, for example. It would want at least all of the rounded corners squared up and for the hollow boxes to be filled in with some darker yellow/brown color, as in the menu and submenu bars. Any style that's overridden in most skins should simply move down to the skin, leaving the default implementation producing semantic HTML, unstyled.

On the latter, I've been waiting for the feature to settle down into stability before making skin changes.

You don't need the parameter: the code can infer which page is requesting style.css and append the necessary extras based on the HTTP Referer header.

Yes, but that level of the code doesn't(?) have(?) access to the request headers. i don't recall (perhaps incorrectly) fossil recording those for downstream use.

(Yes, it's misspelled; accident of history, can't be changed now.)

LOL - i've actually never noticed that, but my textarea's spellchecker just did. That's hilarious.

First, keep in mind that this page should be long-term cached, so it's not like you're actually sending /fileedit page CSS with every page hit. The CSS text only goes out once between edits as long as the user visits the page often enough to keep it in the browser cache.

The fileedit page isn't my concern in that regard: all other pages are. They would currently need to include all of fileedit's CSS on every uncached visit. As of right now that's 7967 bytes of CSS (and the corresponding browser-side higher-level representation of it (i.e. memory cost)) which they won't use.

(Sidebar: before fossil brought me back into the world of C, and made C my favorite language, i had zero concerns about memory costs and related optimizations. Life was so much simpler. ;))

... leaving the default implementation producing semantic HTML, unstyled.

i would argue not necessarily unstyled, but minimally styled, which leaves skins the option, but not the requirement, of customizing it. Forcing each skin to implement 50+ styles (just to pull a number out of the air) just to get off the ground seems excessive.

As of right now that's 7967 bytes of CSS

...which a typical active Fossil user will pay once every 3 or so months.

Negligible.

You have to keep such concerns in perspective: 8k of CSS would be "expensive" if it was inline in every page, but it isn't. It's fully amortized over the user's Fossil upgrade period.

Even for those few of us who upgrade Fossil approximately weekly, it's still a tiny cost to pay, amortized over the many page hits.

There are far more worthy things to worry about than the size of long-term cached CSS. For just one, the fact that js.txt is still injected into every page, not served externally and cached in the same way that style.css is!

Forcing each skin to implement 50+ styles...

...is the maintenance cost of carrying 14 stock skins. Sucks, but it is what it is.

I'm well aware that I'm talking about a lot of work. It has to be done if we're going to maintain the current level of skin beauty.

So, is the feature stable enough to begin reskinning everything, or should we hold off?

Forcing each skin to implement 50+ styles...

...is the maintenance cost of carrying 14 stock skins. Sucks, but it is what it is.

i don't believe it's necessary, though. We might have more skins, or more widespread customization in client repos, if the cost of entry (in the number of selectors and styles which must be implemented) weren't so high.

Admittedly, the current skins already implement all of these rules, and there's no reason to go gut them, but there is a cleaner path forward for future skins and client-side customization of existing ones. Namely, emit the default CSS and derive/cascade as needed, rather than completely override everything up front. That's how CSS is intended to work, rather than fossil's current approach of requiring skins to provide all-or-none style for a given selector.

There are far more worthy things to worry about than the size of long-term cached CSS.

Consider it my personal itch ;).

For just one, the fact that js.txt is still injected into every page, not served externally and cached in the same way that style.css is!

That's something the fileedit branch necessarily addresses as well, using builtin files to store each required JS module and serving them as <script src=...> with the same cache-buster as style.css. If we're going to add ajaxed previews to wiki/forum, we'll need those same features, so style.c was expanded a bit to centralize it and allow selective inclusion of the various fossil.*.js modules.

That infrastructure is all stuck in fileedit's branch until, at a minimum, the 2.11 release, though. (That said, i'm ready to merge fileedit on a moment's notice.)

So, is the feature stable enough to begin reskinning everything, or should we hold off?

i don't currently think there's a need to reskin anything if we take the approach demonstrated in the proof-of-concept branch. All of the current skins implement the various default styles, and i don't expect any notable collisions where a given current skin fails to override all styles of any given selector.

That's a stepping stone towards the next rabbit hole of cleaning up/overhauling the site's CSS in a way which doesn't require wholesale re-skinning, but allows incremental modernization of the builtin skins. The one currently-missing feature which would simplify it is for mkcss.c to support comma-delimited selectors, as we could then support both the historical and hypothetical new CSS without significant CSS rule duplication.

Maybe it is time to take CSS out of fossil in a way that is similar to fossil's embedded documentation. Specify a "skin" directory where each sub-directory is the name of a "skin". Then there would be only one official fossil skin "skin/fossil".

What happens to the other stock skins? They are removed from fossil and perhaps moved into a separate repo or offered as uv tarballs. Maybe use fossil bundle as the skin delivery mechanism. This would remove the burden for maintaining skins from the main fossil developers.

Just a thought from someone who has contributed to one of the stock skins. If the entire CSS landscape of fossil was redone and simplified, I would be extremely happy.

Maybe it is time to take CSS out of fossil in a way that is similar to fossil's embedded documentation.

That's essentially how i do skin CSS overrides in my own repos, rather than edit it in the skin editor's textarea. It's far easier to work with and lets me use emacs' css-mode.

Specify a "skin" directory where each sub-directory is the name of a "skin". Then there would be only one official fossil skin "skin/fossil".

We can't force repos to have another specific directory name, as that may conflict with their structure, but we do already have the .fossil-settings dir, and could place such a thing there, under skin/SKINNAME. style.css could hypothetically then check for (1) that structure in the current checkout (for a checkout) or (2) that structure the current branch of the repo, and push whatever skin/header/footer/etc. file(s) it finds there.

Maybe it is time to take CSS out of fossil in a way that is similar to fossil's embedded documentation. Specify a "skin" directory where each sub-directory is the name of a "skin". Then there would be only one official fossil skin "skin/fossil".

What happens to the other stock skins? They are removed from fossil and perhaps moved into a separate repo or offered as uv tarballs. Maybe use fossil bundle as the skin delivery mechanism. This would remove the burden for maintaining skins from the main fossil developers.

I would welcome this change very enthusiastically.

that level of the code doesn't(?) have(?) access to the request headers.

I just went spelunking, and this particular header is extracted to CGI environment variable HTTP_REFERER in src/cgi.c.

For other cases, you can run a regex on g.httpHeader.

i would argue not necessarily unstyled, but minimally styled,

i've changed my mind on that. The default, skinless output should indeed be unstyled... for a given definition of "unstyled." The site needs a skin, and that part is responsible for applying any non-browser-default style.

That said, the default styles probably should "reset" their elements to some sane defaults. Not colors or font sizes, but margin:0, padding:0, border:none, and similar commonly-seen-in-CSS resets. Lots of sites start their CSS with a "clean slate" by forcing almost everything to some well-known sane defaults, as browsers often differ in their defaults for many properties. Here's an example, from CSS guru Eric Meyer:

https://meyerweb.com/eric/tools/css/reset/

And perhaps that small snippet could comprise all of our default-level CSS, leaving the rest to the skins. Where fossil-defined CSS classes do nothing, not even a clean-slate reset, then there's no point in emitting them at all.

You don't need the parameter: the code can infer which page is requesting style.css and append the necessary extras based on the HTTP Referer header.

That seems like a very strange use of the Referer header. Wouldn't that interfere with caching the CSS?

Also, some people configure their browser to just turn off the Referer header.

Fossil already uses REFERER as part of its Cross-Site Request Forgery (CSRF) defense. For some pages, if the HTTP request does not come from the same origin (as determined by examining the REFERER) then the HTTP request is assumed to be a CSRF attempt and the query or POST parameters are deliberately ignored.

If the user sets network.http.sendRefererHeader to 1, (send the REFERER only when clicking on links and similar elements) then Fossil should still work fine. But if they set it to 0 (never send the REFERER) then some features of Fossil will be unavailable.

But if they set it to 0 (never send the REFERER) then some features of Fossil will be unavailable.

"Some features" would, in this case, include delivery of any page-specific styles. Is that a gamble worth taking? Is CSRF a concern for delivery of CSS? (That's a genuine question - XSS, CSRF, and friends are Greek to me.)

CSRF is not an issue for read-only pages, such as CSS. It is only a concern for pages that have side-effects (for example /fileedit).

If the user sets network.http.sendRefererHeader to 1, (send the REFERER only when clicking on links and similar elements) then Fossil should still work fine. But if they set it to 0 (never send the REFERER) then some features of Fossil will be unavailable.

CSRF is not an issue for read-only pages, such as CSS. It is only a concern for pages that have side-effects (for example /fileedit)

fileedit was updated this morning to honor CSRF, but it turns out that XHR requests do not submit referrer info unless sendRefererHeader is 2. See /forumpost/ecf09a5b3d and its response for details.

Is CSRF a concern for delivery of CSS?

I don't think CSRF is, but XSS attacks can involve CSS. A host like Chisel could have one client injecting CSS into another page underneath the same-origin shield that causes some bit of security-sensitive info to be presented in a masked manner. Or vice versa: an otherwise security-insensitive page that appears to ask for credentials or something, which could cause someone to feed in info that couldn't otherwise be harvested.

If you're not seeing how, consider the :after bit in my initial replies above, causing one page to inject whole HTML forms into another page via CSS.

This is why CSS is one of the things often restricted via CSP.

Here are a few more possibilities.

You don't need the parameter: the code can infer which page is requesting style.css and append the necessary extras based on the HTTP Referer header.

It turns out that we actually do need the parameter when emitting $stylesheet_url. Consider this series of page visits across a repo the site:

• /wiki requests /style.css?id=... (no per-page style)
• /forum requests /style.css?id=... (no per-page style)
• /fileedit requests /style.css?id=... (doh - per-page style)

When the browser hits /fileedit it sees the same old style.css path as it's been seeing for this whole session, it will use a cached copy of the style. (Remember that the id parameter is specific to the fossil build, not the CSS content.)

Thus we need:

• /wiki requests /style.css?id=... (no per-page style)
• /forum requests /style.css?id=... (no per-page style)
• /fileedit requests /style.css/fileedit?id=... (per-page style)

However, style.css now ignores the P("name") value, which is necessarily there for cache-busting, and uses the refer[r]er to determine which, if any, page-specific CSS to serve.

My idea was that /fileedit (or any other page that requires custom CSS) would request something other than /style.css in its <link> header markup.

• /wiki → /style.css?id=...
• /forum → /style.css?id=...
• /fileedit → /perpage-style.css?id=...

The /style.css page works as it always has, returning default_css.txt. But /perpage-style.css page looks at the g.zPath and possibly returns a different result, depending on g.zPath. To make caching work, the id= value for /perpage-style.css would likely need to include the g.zPath as well.

But, after thinking about it, I suppose just adding a page= query parameter to /style.css in cases where you need a custom CSS would work just as well.

The /style.css page works as it always has, returning default_css.txt. But /perpage-style.css page looks at the g.zPath and possibly returns a different result, depending on g.zPath. To make caching work, the id= value for /perpage-style.css would likely need to include the g.zPath as well.

That approach has an ordering problem, but not one we can't overcome: the per-page rules ideally need to come between the defaults and the skin, so that the defaults don't override them and the skin can override them. That can be worked around by making sure the per-page rules use, where necessary, selectors which are more specific than the defaults, so that the defaults do not override them (perhaps inadvertently, via a class name which is the same but is used for a different purpose - i managed to create such a collision early on in fileedit, then added body.fileedit to all of its selectors to force them to be more specific than the defaults).

The more onerous problem is that in order to implement a separate CSS URL we have to change all skins to dynamically emit all of the <link> tags they require, as opposed to just emitting $stylesheet_url. That's not a technical hurdle, but would have fallout in client-side skins, requiring either editing or resetting of skins to work. My preference (but not a strong one) for that specific change would be to wait until the next major release, when we can get away with making any number of massive changes at the same time.

But, after thinking about it, I suppose just adding a page= query parameter to /style.css in cases where you need a custom CSS would work just as well.

Either way is perfectly fine with me. The current path of least resistance, in terms of avoiding any skin-level changes (at least for client-side skins which already use $stylesheet_url), is to use some variant of style.css/pagename or style.css?page=.... They're the same in both function and the amount of code, so whichever you prefer.

As part of the larger proposal of a CSS overhaul, it may make more sense to split it into multiple <link> tags, as such an overhaul seems at least marginally likely to have other fallout which will require resetting/edits of skins, anyway.

Can the style sheet that $stylesheet_url references not simply be the concatenation of all of the style sheets that the page requires? Isn't that what the current page= mechanism does?

Can the style sheet that $stylesheet_url references not simply be the concatenation of all of the style sheets that the page requires? Isn't that what the current page= mechanism does?

Correct. That's what the style-css-revamp branch does, and what fileedit does (but it works differently there, for compatibility with trunk).

The primary difference between trunk and style-css-revamp is that trunk conditionally outputs the default CSS selectors depending on whether it finds them in the skin CSS. The revamp instead first unconditionally outputs all of the defaults, then outputs any page=xyz style, then outputs the skin.

Yesterday i tested the revamp will all of the built-in skins, going through all of the pages in the main menu and a random selection of other pages, and saw not a single visual difference between both approaches. That leaves me feeling confident that the revamp approach (which is precisely what page-specific rules need, in terms of CSS ordering and cascading) has zero, or at most very close to zero, compatibility impact for us vis a vis existing client skins. :)

However, style.css now ignores the P("name") value, which is necessarily there for cache-busting, and uses the refer[r]er to determine which, if any, page-specific CSS to serve.

That was, IMO, a mistake, as it depends on (completely reasonably) browser-level settings which fossil cannot reliably influence. i'm going to revert that to one of style.css?id=...&page=xyz or style.css/xyz?id=..., whichever you guys prefer (the latter looks nicer to me but they're equivalent semantically and code-wise, so whichever is fine with me). The referrer is simply too unreliable and fragile for infrastructure-level read-only stuff like the stylesheet, but we can arguably rightfully demand it for POST/write-operations.

OK

I'm not sure if we're trying to accommodate specific CSS for /fileedit page or generally split-off page-specific styling from main css.txt.

Fossil webpages are mostly /dirnames, that is there's no filename.html. So for a page-specific CSS file I would expect a file with page-specific path:

http://host/fossil/webpage/style.css

A skin could supply its own version of classes for a given webpage by including a 'webpage/css.txt' in its subtree. For example, the original skin to add specific CSS for /info page, it could add a file:

skins/original/info/css.txt

The Fossil templator would need to check if there's a loaded resource for a given webpage request and add the page-specific style.css line.

Technically, nothing stops a skin from overriding specific classes in its the main css.txt file.

I'm not sure if we're trying to accommodate specific CSS for /fileedit page or generally split-off page-specific styling from main css.txt.

fileedit is the first page it is being addressed for, but i would eventually like to see all pages with significant amounts of page-specific CSS do it (the timeline comes to mind). Baby steps.

So for a page-specific CSS file I would expect a file with page-specific path:

The "problem" with that approach is that fossil's dispatcher doesn't support it. Internally it handles only one level of directory name for dispatching purposes, and treats anything after that one part as a single token which gets assigned to the "name" CGI argument. Thus page/foo/bar and page?name=foo/bar are equivalent once fossil's innermost internals get done processing the request path and arguments, before the page is dispatched.

Alternative is to parameterize the style.css to the page route.

http://host/fossil/webpage?style.css=$id

Alternative is to parameterize the style.css to the page route.

http://host/fossil/webpage?style.css=$id

That's essentially what the style-css-revamp branch is doing, except that it's parameterizing it on /style.css/pagename. Swapping that relationship and handling it at the page level would require that every link to that page also pass on the hypothetical style.css parameter, which isn't realistic. The <LINK> tag which imports style.css is output in only one place: when the page's skin is rendered. At that point it knows the current page and can transparently amend the CSS with any per-page CSS.

Swapping that relationship and handling it at the page level would require that every link to that page also pass on the hypothetical style.css parameter, which isn't realistic.

That's completely false - that link is still only needed on the page's header, but the current approach is simpler.

With the initial style.css revamp now in trunk, the current process of internally storing each rule of default_css.txt separately in a struct is effectively obsolete.

If there are no objections, i would like to move default_css.txt into a builtin file named default.css. That would simplify maintenance by allowing use of CSS-aware editors (which don't like the C-style comments that file currently uses) and would simplify the pending CSS revamp by allowing us to combine "old" and "new" style names into single definitions, e.g.:

timelineHistDsp, /* old */
timeline-history-display /* new */
{ ...rules... }

Which, in turn, would allow us to normalize/modernize all CSS class names without breaking any 3rd-party skins. We could then remove the old names at 3.0.

The current default_css.txt approach requires that those be two separate rules, as it doesn't understand comma-separated selectors (edit: it understands them, but treats them as one rule, rather than separate rules).

If there are no objections, i would like to move default_css.txt into a builtin file named default.css.

Here's what that would look like:

https://fossil-scm.org/home/timeline?r=default.css

Here's what that would look like:

https://fossil-scm.org/home/timeline?r=default.css

If there are no objections to this change, i'd like to merge that into trunk in the next day or so.

Summary: move default_css.txt to default.css, as recent changes have obsoleted the need for the former, and remove mkcss.c, as it's no longer needed/applicable. This will make working with default.css easier (at least for people who use CSS-aware tools).

If there are no objections to this change, i'd like to merge that into trunk in the next day or so.

Summary: move default_css.txt to default.css, as recent changes have obsoleted the need for the former, and remove mkcss.c, as it's no longer needed/applicable.

Just FYI, that's now in trunk.