Discussion:
[minor] src/style.c:427 → "%<body%" check
(too old to reply)
mario
2018-07-02 14:11:04 UTC
Permalink
This misses anything but plain <body> tags in the header

if( sqlite3_strlike("%<body>%", zHeader, 0)!=0 ){
Th_Render(zDfltHeader);
}

It might rather be %<body% or %<body%>%, so any style attributes
like <body class="PageyMcPageface"> get recognized still.

Perhaps zDfltHeader[] could even contain a short HTML comment as to
why it was injected. Took me an hour two figure out how the CSP came
to be^^

##

[skin-setup-refactor]. I see the value in the draft feature, but it's
also a bit confusing still (while working on broken skins at least.)

Can we have an option to hide draft admin, or the setup_skin+_admin
pages merged with e.g.:
- draftN… just treated as saved skins?
- edit header/footer/css buttons for each draft/skin
- and [test] urls for each available backup/save/draft
Or something like that.


G!
Warren Young
2018-07-02 23:20:17 UTC
Permalink
Post by mario
This misses anything but plain <body> tags in the header

if( sqlite3_strlike("%<body>%", zHeader, 0)!=0 ){
Th_Render(zDfltHeader);
}
It might rather be %<body% or %<body%>%, so any style attributes
like <body class="PageyMcPageface"> get recognized still.
Under what conditions would you have two different <body> tags in a single document differing only by class, and thus need a CSS selector to differentiate them?

I suspect this is just a bad example, but then I ask myself, if I’m going to apply CSS styles to a tag, wouldn’t I just put it into the CSS file, which I can already modify without changing Fossil?

What’s the real motivation here?
Post by mario
I see the value in the draft feature, but it's
also a bit confusing still (while working on broken skins at least.)
While I agree that the new skin editor is considerably more complicated to use than the pre-2.5 method, I also don’t see how to make it simpler without losing functionality we gained in that release.

This feature set solves a few real problems faced by drh during the 2.5 development sequence in support of the many UI improvements he made while actively soliciting feedback on those changes. In order to get that feedback, he had to point people to a running instance, so rather than take the expense and complication of setting up a public demo server, drh created this feature set so he could demonstrate proposed skin changes using the fossil-scm.org instance to get that feedback.

This feature set adds the following abilities:

1. Lets normal site visitors A/B compare proposed changes by visiting URLs differing only in the skin draft’s name.

2. The Fossil admin user can now invite others to edit the skin.

3. Edits can be made to drafts, so you can test on a production server without breaking it for uninterested users.

The prior way skin editing worked, only the admin user could switch and edit skins.

I don’t think you’re going to see any changes to this mechanism unless your proposal can achieve the same ends.
Post by mario
- draftN… just treated as saved skins?
I don’t see why that couldn’t be done in principle.

The main practical problem is that skins can be named in a way that would require ugly URL escaping to be specified in place of the current draftN bit:

https://fossil.example.com/Blitz%2C%20No%20Logo%20(built-in)/home

A secondary problem is that it appears that Fossil’s current URL parser can’t quite cope with that once decoded, based on the 404 error pages it generates.

There is a side-benefit of this change: if someone has a public Fossil instance and one of the end users thinks the default skin is ugly, he can change it by selecting the name of a skin he likes better in the URL. Currently, you can only change the skin this way in a local clone which you’re running “fossil ui” on.
Post by mario
- edit header/footer/css buttons for each draft/skin
I’m not seeing the value of that change. The current mechanism is:

1. Select the skin/draft to edit.

2. Click the CSS/Header/Footer/Details edit links.

Are you proposing to replace this with a grid, with skin names down the left edge and 4 edit buttons per row to the right of each skin name? That does not sound like a simplification to me.

If you have something different in mind, a graphical mockup or wireframe drawing might help get your point across.
Post by mario
- and [test] urls for each available backup/save/draft
We already have that: it’s Step 5 on the setup_skin page. The test URL list changes whenever you change the draft name in Step 1.
mario
2018-07-03 03:23:42 UTC
Permalink
Post by Warren Young
Under what conditions would you have two different <body> tags in a
single document differing only by class, and thus need a CSS selector
to differentiate them?
Of course you wouldn't want two <body> tags.
But that's exactly the bug I ran into:


<!DOCTYPE html>
<html>
<head>
<base href="http://localhost:8080/timeline" />
<meta http-equiv="Content-Security-Policy" content="default-src 'self'
data: 'unsafe-inline'" /> <title>Plugin Meta Data: Timeline</title>
<link rel="alternate" type="application/rss+xml" title="RSS Feed"
href="/timeline.rss" /> <link rel="stylesheet"
href="/style.css?id=6f242840" type="text/css" media="screen" /> </head>
<body>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<base href="http://localhost:8080/timeline" />
<title>Plugin Meta Data: Timeline</title>
<link rel="stylesheet" href="/style.css?id=6f242840" type="text/css">
<link rel="stylesheet"
href="https://media.readthedocs.org/css/sphinx_rtd_theme.css"
type="text/css" /> <link rel="stylesheet"
href="https://media.readthedocs.org/css/readthedocs-doc-embed.css"
type="text/css" />
<!--script type="text/javascript"
src="https://sphinx-rtd-theme.readthedocs.io/en/latest/_static/readthedocs-data.js"></script-->
</head>
<body class="wy-body-for-nav" role="document">


If you use a skin header with `<body and=attributes>`, current trunk
Post by Warren Young
/*
** Default HTML page header text through <body>. If the
repository-specific ** header template lacks a <body> tag, then all
of the following is ** prepended.
*/
It should check for <body%>, not just a fixed byte sequence.

##
Post by Warren Young
Post by mario
I see the value in the draft feature, but it's
also a bit confusing still (while working on broken skins at
least.)
While I agree that the new skin editor is considerably more
complicated to use than the pre-2.5 method, I also don’t see how to
make it simpler without losing functionality we gained in that
release.
TBH, I haven't really given it much more thought.

It's clearly more practical to have secondary skins to work on,
without harming the active/default template. And being able to
grant additional edit permissions there is a nifty feature. As
is the built-in help and step-by-step guide.

It just got in the way for the aforementioned issue.
Post by Warren Young
Post by mario
- draftN… just treated as saved skins?
I don’t see why that couldn’t be done in principle.
The main practical problem is that skins can be named in a way that
would require ugly URL escaping to be specified in place of the
https://fossil.example.com/Blitz%2C%20No%20Logo%20(built-in)/home
If the PATH_URI detection can only only recognize `draftN` prefixes,
than having all of them available wouldn't make sense. With the ugly
percent escaping anyway.
Post by Warren Young
There is a side-benefit of this change: if someone has a public
Fossil instance and one of the end users thinks the default skin is
ugly, he can change it by selecting the name of a skin he likes
better in the URL. Currently, you can only change the skin this way
in a local clone which you’re running “fossil ui” on.
Rather than extending the internal path dispatcher, supporting a
plain/per-user cookie migth make sense. -- OTOH, this might already
be feature creep..
Post by Warren Young
1. Select the skin/draft to edit.
2. Click the CSS/Header/Footer/Details edit links.
Are you proposing to replace this with a grid, with skin names down
the left edge and 4 edit buttons per row to the right of each skin
name? That does not sound like a simplification to me.
It's sort of what came to mind. But indeed, that would probably just
make it more convoluted.

One simplification in the whole skin conundrum could be merging
css+header+footer+settings editing into one page (4x textarea)
however.
Then it might be presentable in a table/grid scheme:

check test
boxes links
↓ ↓
[ ] Skin name [Edit]
[x] Backup2 [Edit]
[ ] Draft1 [Edit]

[Activate] [Use as Draft] [Publish] [Delete]

With the buttons just acting on the current selection.

* Could be fewer action buttons though.
* [Rename] or [Publish] better part of the css+head+foot editing
page.
* The `Authenticate` option should really be part of the `Display
Details` settings blob, btw.
* It would likely suffice if the template name doubles as test link
(don't see the need for five test links).
Post by Warren Young
If you have something different in mind, a graphical mockup or
wireframe drawing might help get your point across.
I'm gonna give this a bit more thought. There's quite a lot of
backend code for the template management already. Not really sure
if any of this would simplify code or editing workflows.
David Mason
2018-07-03 03:38:16 UTC
Permalink
It's pretty common to put classes on <body> tags, to use CSS to
conditionally choose different renderings by simply changing the class of
the body tag.

../Dave
Post by mario
Post by Warren Young
Under what conditions would you have two different <body> tags in a
single document differing only by class, and thus need a CSS selector
to differentiate them?
Of course you wouldn't want two <body> tags.
<!DOCTYPE html>
<html>
<head>
<base href="http://localhost:8080/timeline" />
<meta http-equiv="Content-Security-Policy" content="default-src 'self'
data: 'unsafe-inline'" /> <title>Plugin Meta Data: Timeline</title>
<link rel="alternate" type="application/rss+xml" title="RSS Feed"
href="/timeline.rss" /> <link rel="stylesheet"
href="/style.css?id=6f242840" type="text/css" media="screen" /> </head>
<body>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<base href="http://localhost:8080/timeline" />
<title>Plugin Meta Data: Timeline</title>
<link rel="stylesheet" href="/style.css?id=6f242840" type="text/css">
<link rel="stylesheet"
href="https://media.readthedocs.org/css/sphinx_rtd_theme.css"
type="text/css" /> <link rel="stylesheet"
href="https://media.readthedocs.org/css/readthedocs-doc-embed.css"
type="text/css" />
<!--script type="text/javascript"
src="https://sphinx-rtd-theme.readthedocs.io/en/latest/_
static/readthedocs-data.js"></script-->
</head>
<body class="wy-body-for-nav" role="document">
If you use a skin header with `<body and=attributes>`, current trunk
Post by Warren Young
/*
** Default HTML page header text through <body>. If the
repository-specific ** header template lacks a <body> tag, then all
of the following is ** prepended.
*/
It should check for <body%>, not just a fixed byte sequence.
##
Post by Warren Young
Post by mario
I see the value in the draft feature, but it's
also a bit confusing still (while working on broken skins at least.)
While I agree that the new skin editor is considerably more
complicated to use than the pre-2.5 method, I also don’t see how to
make it simpler without losing functionality we gained in that
release.
TBH, I haven't really given it much more thought.
It's clearly more practical to have secondary skins to work on,
without harming the active/default template. And being able to
grant additional edit permissions there is a nifty feature. As
is the built-in help and step-by-step guide.
It just got in the way for the aforementioned issue.
Post by Warren Young
Post by mario
- draftN
 just treated as saved skins?
I don’t see why that couldn’t be done in principle.
The main practical problem is that skins can be named in a way that
would require ugly URL escaping to be specified in place of the
https://fossil.example.com/Blitz%2C%20No%20Logo%20(built-in)/home
If the PATH_URI detection can only only recognize `draftN` prefixes,
than having all of them available wouldn't make sense. With the ugly
percent escaping anyway.
Post by Warren Young
There is a side-benefit of this change: if someone has a public
Fossil instance and one of the end users thinks the default skin is
ugly, he can change it by selecting the name of a skin he likes
better in the URL. Currently, you can only change the skin this way
in a local clone which you’re running “fossil ui” on.
Rather than extending the internal path dispatcher, supporting a
plain/per-user cookie migth make sense. -- OTOH, this might already
be feature creep..
Post by Warren Young
1. Select the skin/draft to edit.
2. Click the CSS/Header/Footer/Details edit links.
Are you proposing to replace this with a grid, with skin names down
the left edge and 4 edit buttons per row to the right of each skin
name? That does not sound like a simplification to me.
It's sort of what came to mind. But indeed, that would probably just
make it more convoluted.
One simplification in the whole skin conundrum could be merging
css+header+footer+settings editing into one page (4x textarea)
however.
check test
boxes links
↓ ↓
[ ] Skin name [Edit]
[x] Backup2 [Edit]
[ ] Draft1 [Edit]
↓
[Activate] [Use as Draft] [Publish] [Delete]
With the buttons just acting on the current selection.
* Could be fewer action buttons though.
* [Rename] or [Publish] better part of the css+head+foot editing
page.
* The `Authenticate` option should really be part of the `Display
Details` settings blob, btw.
* It would likely suffice if the template name doubles as test link
(don't see the need for five test links).
Post by Warren Young
If you have something different in mind, a graphical mockup or
wireframe drawing might help get your point across.
I'm gonna give this a bit more thought. There's quite a lot of
backend code for the template management already. Not really sure
if any of this would simplify code or editing workflows.
_______________________________________________
fossil-users mailing list
http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users
Warren Young
2018-07-03 14:50:20 UTC
Permalink
It's pretty common to put classes on <body> tags, to use CSS to conditionally choose different renderings by simply changing the class of the body tag.
I think you’d have to write TH1 code to get Fossil to serve more than one <body> tag on a given repository.

That then makes me wonder if that would be another way to trick Fossil into serving a second <body> tag. Consider this pseudocode:

<th1>
return [concat "<body" [somefn $args] ">"]
</th1>

I say “pseudocode” because it’s probably horrid Tcl style, if it compiles at all. I speak only pidgin Tcl these days.
David Mason
2018-07-04 03:12:02 UTC
Permalink
Sorry if I was confusing... You'd only have one body tag, but the class
might change while the page is loaded... and it might be initialized to
something.

You might have CSS like:
---------
div.first, div.second, div.third {display: none}
body.start div.first,
body.next div.second,
body.last div.third {display: block}
----------
Then a body like: <body class="start">

And some Javascript that did: document.body.className="next";

Note that https://developer.mozilla.org/en-US/docs/Web/HTML/Element/body
points out that you can only have one body in a valid HTML page.

../Dave
Post by David Mason
Post by David Mason
It's pretty common to put classes on <body> tags, to use CSS to
conditionally choose different renderings by simply changing the class of
the body tag.
I think you’d have to write TH1 code to get Fossil to serve more than one
<body> tag on a given repository.
That then makes me wonder if that would be another way to trick Fossil
<th1>
return [concat "<body" [somefn $args] ">"]
</th1>
I say “pseudocode” because it’s probably horrid Tcl style, if it compiles
at all. I speak only pidgin Tcl these days.
_______________________________________________
fossil-users mailing list
http://lists.fossil-scm.org:8080/cgi-bin/mailman/listinfo/fossil-users
Loading...