Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

class name conflicts in wallabag's css #7180

Open
HolgerAusB opened this issue Jan 14, 2024 · 6 comments
Open

class name conflicts in wallabag's css #7180

HolgerAusB opened this issue Jan 14, 2024 · 6 comments
Labels

Comments

@HolgerAusB
Copy link

Environment

  • Version: 2.6.7
  • Installation: GIT

What steps will reproduce the bug?

Recently I saw several websites which uses the same class names as they are in wallabag's material.css which caused conflicts in the visualization. So I needed to string_replace these class-names in the site specific configs. But this could lead to problems if these class names appear in the normal body text of the article.

Suggestion

So we need either more unique classnames as wallabag-text-primary instead of text-primary

or

we need a crawler that is renaming all class (and id?) elements from fetched html which are used in our css files: e.g.
text-primary -> notwallabag-text-primary

I'd prefer the first option, because the second one might break some site rules

Examples (all fixed by string replacement in site config)

* usbeketrica.com

uses .text-primary. That results in a white text-color, which goes invisible on the light-theme

* apple.com

uses .video-container which causes wallabag to enlarge the element, while the video could not be embed.
e.g. https://www.apple.com/newsroom/2023/10/apple-supercharges-24-inch-imac-with-new-m3-chip/

* telepolis.de

uses <h3 class="subheading">. While this class is not part of materials css, these subheaders where stripped from wallabag. After replacing the class in site config they remain in text. So might be a different problem here.

* next.ink

uses <h3 class="wp-block-heading">. This class is also not part of material.css but subheadings get stripped by wallabag.

@j0k3r j0k3r added the UI label Jan 16, 2024
@j0k3r
Copy link
Member

j0k3r commented Jan 16, 2024

That's a good point.
Is it easily doable @Simounet?

@yguedidi
Copy link
Contributor

just curious, I'm wondering why do we have remaining classes from the original content? isn't the fetch content stripped from all non necessary things to be displayed inside wallabag in "reader mode"?

@Simounet
Copy link
Member

It's doable but like @yguedidi I think it could be great to strip all classes before storing the content into our db.

@HolgerAusB
Copy link
Author

hmmm, I see what you mean, @Simounet. But on the other hand, it could be helpful to keep them, if somebody uses custom.css. So I wrote a still dirty mastodon-wrapper (python cgi), which get toots including all ascendants and descendants via api and construct a plain html out of this, so I can catch that with wallabag. And I am pushing own classes for some elements to build a nicer view to that 'articles'. That would be gone then.

Maybe we could store store the classes in the db but have a toggle to optionally remove them when reading from db and showing in the UI and/or API. Maybe with persistent toggle per article or domain.

@HolgerAusB
Copy link
Author

And one more point. For debugging my site-configs it is sometimes helpfull to inspect the html-output of wallabag, to find out, which classes/ids should be striped or preserved/selected or string_replaced. That means, that I need the original classes and stuff at least as an optional view.

@HolgerAusB
Copy link
Author

...maybe it could also/instead be helpfull to have a special strip or replace directive in site-depended configs?

e.g.

strip_classname: container
strip_classname: text-*      #wildcard
replace_classname(subheading): foosubheading
prefix_classname(*-video): foobar-     #wildcard

# this one would rename/prefix all classnames from original article:
#prefix_classname(*): siteoriginal-

so the source

<div class="first second container third text-primary fourth">
   <h2 class="main subheading extralarge">
      Super cool heading text
   </h2>
   <div class="dark mytube-video boxed"
      ...
   </div>
</div>

could be rewritten as

<div class="first second third fourth">
   <h2 class="main foosubheading extralarge">
      Super cool heading text
   </h2>
   <div class="dark foobar-mytube-video boxed"
      ...
   </div>
</div>

Maybe same with id-names?

What do you think @j0k3r, @nicosomb @Simounet, @yguedidi, @Kdecherf, @fivefilters (for FTR)?

another idea:

prefix_all_classnames: yes|no   #standard = no
prefix_all_idnames:    yes|no   #standard = no

while the product is choosing a fix prefix, e.g.:

wallabag:    container => wlbgx_container
Fivefilters: container => 5fltrx_container
Graby:       container => grabyx_container

which could enable easier product specific handling in one config file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants