Reviewers
Request review
No reviewers
標籤
清除已選取標籤
Related to the REST API
Code running on the server
Stuff related to Federation
Related to the front-end
Translations, and related code
More about project management or code than the project itself
The building, or installation process of Plume
Something isn't working
We need to talk
New feature or request
This is a new feature
Compatibility with different browsers, readers and OS
Related to an external package that Plume uses
UI/UX related issues and PRs
Good for newcomers
Extra attention is needed
Issues affecting only mobile UX
How elements're rendered out for the end user
Something else needs to be fixed first
This issue or pull request already exists
This PR is not complete yet
Issues concern a limited number of instances
This doesn't seem right
Need to be discussed by the community (on Loomio)
This PR is ready to be reviewed
Proposed ideas worth considering
This is issue has been created after a vote on Loomio
This will not be worked on
Apply labels
A: API
Related to the REST API
A: Backend
Code running on the server
A: Federation
Stuff related to Federation
A: Front-End
Related to the front-end
A: I18N
Translations, and related code
A: Meta
More about project management or code than the project itself
A: Security
Build
The building, or installation process of Plume
C: Bug
Something isn't working
C: Discussion
We need to talk
C: Enhancement
New feature or request
C: Feature
This is a new feature
Compatibility
Compatibility with different browsers, readers and OS
Dependency
Related to an external package that Plume uses
Design
UI/UX related issues and PRs
Documentation
Good first issue
Good for newcomers
Help welcome
Extra attention is needed
Mobile
Issues affecting only mobile UX
Rendering
How elements're rendered out for the end user
S: Blocked
Something else needs to be fixed first
S: Duplicate
This issue or pull request already exists
S: Incomplete
This PR is not complete yet
S: Instance specific
Issues concern a limited number of instances
S: Invalid
This doesn't seem right
S: Needs Voting/Discussion
Need to be discussed by the community (on Loomio)
S: Ready for review
This PR is ready to be reviewed
Suggestion
Proposed ideas worth considering
S: Voted on Loomio
This is issue has been created after a vote on Loomio
S: Wontfix
This will not be worked on
未選擇標籤
A: API
A: Backend
A: Federation
A: Front-End
A: I18N
A: Meta
A: Security
Build
C: Bug
C: Discussion
C: Enhancement
C: Feature
Compatibility
Dependency
Design
Documentation
Good first issue
Help welcome
Mobile
Rendering
S: Blocked
S: Duplicate
S: Incomplete
S: Instance specific
S: Invalid
S: Needs Voting/Discussion
S: Ready for review
Suggestion
S: Voted on Loomio
S: Wontfix
里程碑
Set milestone
清除已選取里程碑
No items
未選擇里程碑
Assignees
Assign users
Clear assignees
No Assignees
2 參與者
訊息
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.
No due date set.
Dependencies
No dependencies set.
Reference: Plume/Plume#624
Reference in New Issue
尚未有任何內容
Delete Branch 'themes'
Deleting a branch is permanent. It CANNOT be undone. Continue?
取消操作
確認操作
Instance level themes are just CSS files, stored in
static/css
(seeInstance::list_themes
).TODO:
Fixes #354, #403
Codecov Report
❓
@ -0,0 +1,4 @@
-- Your SQL goes here
ALTER TABLE blogs ADD COLUMN theme VARCHAR;
ALTER TABLE users ADD COLUMN preferred_theme VARCHAR;
this should be a cookie setting
I really hope this is a cookie setting.
@ -0,0 +1,4 @@
-- Your SQL goes here
ALTER TABLE blogs ADD COLUMN theme VARCHAR;
ALTER TABLE users ADD COLUMN preferred_theme VARCHAR;
Why should it be a cookie setting? Storing it in the database as the advantage of having it synced across devices (and also it will never expire).
@ -0,0 +1,4 @@
-- Your SQL goes here
ALTER TABLE blogs ADD COLUMN theme VARCHAR;
ALTER TABLE users ADD COLUMN preferred_theme VARCHAR;
you'd have to sync it to the cookie, but that way it'd be easier & quicker to access.
@ -0,0 +1,4 @@
-- Your SQL goes here
ALTER TABLE blogs ADD COLUMN theme VARCHAR;
ALTER TABLE users ADD COLUMN preferred_theme VARCHAR;
The whole user struct is already loaded for every page anyway, I'm not sure it is going to make a real difference 🤷♀️
is it still ready for review considering the design flaw pointed regarding arbitrary css security?
No, I'm implementing another approach, I think I will be able to push it this evening.
If you want to review this PR, the new version is there. Admins should now approve remote blog themes, but they may contain anything. Once a theme is approved, it is also available for use on this instance. Admins can also upload themes if they have SSH acces to their instance: they just need to create a folder called "blog-SOMETHING" in
static/css
, and add the theme files inside.I will update the docs too.
Does it really make sense to have admins approve remote blog themes? Using remote resources on the local site seems fragile and prone to many more errors in the future. I think it makes more sense to limit themes to site-wide and local-user.
More generally, this is an issue that has to do with the way articles federate in Plume -- the HTML is delivered and cached to many different sites that may have varying CSS. Federating CSS as well is, I think, out-of-scope of ActivityPub (although there is nothing preventing this). In any case, it is much simpler to say that users should view the original URL if they wish to read the article with the original CSS.
I'm not sure this is an actual concern but how does it play with caching? Where are the file stored and are they given a unique name (hash based?)?
The themes are stored in
static/css
so they should integrate with the current caching system I think?I think they are cached, but not evicted when modified, so client might think the file was not updated when it was. Adding a timestamp, a hash, a random string or anything to the name (
myfile-DEADBEEF1312.css
) would fix itI think it is going to be quite difficult, because in the current implementation, the main theme file should be named
theme.css
, and it may load other files of the theme, so we would have to find a way to detect the main theme file even if it has a hash in its name, and to edit allurl()
/@import
inside of it to make sure other files are correctly loaded too… Maybe we can just set a default cache duration of one week or something like that, with a HTTP header? Or is it a problem not to have the latest theme update for a few days?Even 7 days is quiet long and will definitely be reported as a bug at some point. I'm at work currently, maybe we can discuss of it later on riot?
OK, feel free to ping me when you want to talk about it :)
👀
????
@ -0,0 +83,4 @@
last_fetched_date,
fqn,
summary_html
FROM users;
sqlite migration mi amor
in case of error, return empty
Vec
? I think returning aResult<...>
would make more sens, to have a proper error in case of.. errorSame goes
so this whole function was useless? Some other must be too then
Yes, I would like to do a big code cleanup one day, removing all
pub
, progressively reintroducing them until it compiles, and remove everything that Rust says is unused.lgtm
Reviewers
a6c84daa1a
.Step 1:
From your project repository, check out a new branch and test the changes.Step 2:
Merge the changes and update on Forgejo.