Switch Font Awesome Icons from Web Font to SVG

[saxamaphone69]

This is the pull request following the discussion in #1473.

The commits themselves are not as descriptive as they should be, I apologise. Not sure if that is something that can changed.

I think my linter in Atom removed some trailing spaces in some files as well.

TO-DO

• experimented with SVG icons in the embedder, need to revert that
• the settings close button, along with any style changes
• the remove/clear thread icon in the thread watcher, along with any style changes
• check catalog
• add a brief mention of Font Awesome in the Icon object
• write up a list of changes for the wiki
• test on something non-chrome (firefox, etc)
• test with noscript/ublock enabled (i.e. do they allow svgs)
• quick md5 filter uses .fa-times

[ccd0]

4chan-x/Makefile

Lines 54 to 55 in 079d1a5

	imports_src/css/CSS.js := \
	node_modules/font-awesome/fonts/fontawesome-webfont.woff

This should be removed from the Makefile now that the file is no longer being used.
Should fix the No rule to make target 'node_modules/font-awesome/fonts/fontawesome-webfont.woff', needed by '.events/css-CSS.js.js' errors.

[ccd0]

Also I just added one more icon to the header (fa-bolt) for image preloading.

[saxamaphone69]

I've made the changes to Makefile, made the comment on Icon.coffee, added the bolt SVG, and pulled the changes from master, but now it won't let me push anymore.

Even doing git push https://github.com/saxamaphone69/4chan-x.git svgicons -f isn't working for me. I've spent the last 30 minutes trying to push these changes I made 😢

Edit: I think I've just deleted all these changes with that -f force, because now I don't have anything on my local copy...

[ccd0]

What error is it giving you?

[saxamaphone69]

I must have done something with that merge remote-tracking branch....

I was getting errors like fatal: "paths with -a does not make sense", or it was telling me everything was up to date.

I'm trying to revert it back to how it was yesterday night, but the Icons folder in my local copy is not there like it was...

[ccd0]

git reset --hard commitought to revert everything back to a given commit.

[saxamaphone69]

I think my issue was that I never actually added those SVGs with git add or whatever I needed, so all the code references my changes but if you look here:
600a8bb
there's no like "icons.coffee and 25 icons were added" in the changes.

So now that I've done something in my attempt to fix what wasn't broken, I've lost them and will have to redo all those SVGs.

[ccd0]

Would be good to have a script that does them anyway, if just to document the process. I usually put stuff like that in tools/.

[saxamaphone69]

Here it is as a gist then, since I've ruined this PR.

I agree, a tool to do this would be awesome. I did it manually because I'm stupid.

I'll be honest, got no clue how to go about making it.

I mean, I think I get the theory behind it:

1. Use https://github.com/encharm/Font-Awesome-SVG-PNG, especially this option to generate the SVGs from FA4.7.0.
2. Run the SVGs through this bit of JS to fix up the viewBox (a for svg in svgs loop?)
3. Add in the inline-svg class to svg, the data-icon attribute to identify icons, remove the width and height attributes, and add fill="currentColor" to path
4. Optimise the SVGs using svgo
5. It then auto fills the Icon object with icon_name: icon_name.svg` for each icon in the folder.

That might take my small brain a little while to process.

[ccd0]

Can't you just copy the SVGs out of the .user.js file if you still have it?

[saxamaphone69]

I did that (have to remove the escaping \ characters), but as you said, a tool that did all the processing during the build process would make a lot more sense than doing that 5 steps manually. Because say for example the bolt icon, I had to go to that GitHub repo, get the black bolt.svg, add the attributes, run it through the viewBox thing, then run it through a GUI for svgo, then manually open Icons and add it both as a file and as an object.

[saxamaphone69]

Okay, let's pretend that didn't take me over 2 hours to solve.

[saxamaphone69]

And if nothing else, perhaps as mentioned in the original issue, fontello could always be used to include a webfont of only the required icons, which would only reduce the size of the blob and leave everything else unchanged.

[ccd0]

In #1473 you linked a good summary of the advantages of SVGs over icon fonts:
https://css-tricks.com/icon-fonts-vs-svg/
The "Weird Failures" and "Forced Failures" of webfonts it mentions have occasionally been an issue.

The main disadvantage of SVGs as far as 4chan X is concerned is that you can't easily replace them with CSS (you could use background images but you lose the ability to style the SVGs). But even if we switch to SVG, it would be possible to replace them with an icon font by setting the SVG to display: none and setting the content of the parent to the desired text in the desired font. Moving the data-icon from the SVGs to their parents might help.

[saxamaphone69]

It could add bloat, but what about another section in the settings where it's a table of the icons on the left, and the SVG string on the right. That would give people the ultimate control - if someone wants FA4.7, then that's the default. If people want FA5 or Material Icons or the new Bootstrap icons, or literally any SVG icon, they could do that too. Then through Custom CSS they would be able to customise whatever needed to be changed (size, colours).

[ccd0]

I thought about that, but it would break a security policy of mine. User settings could potentially be tainted by a malicious site or a site with an XSS exploit, so I don't allow anything in user settings to execute JavaScript without a user action triggering it (e.g. clicking a link).

[saxamaphone69]

Is this article of any relevance/help?

I think I get what you're saying. Where it would store something in the JSON file like

icons {
'eye':'<svg...>',
'wrench':'<svg...>'
}

and then when 4chan X is building the page, when it wants to add the eye icon, it checks the settings' JSON file to grab the appropriate SVG. But if someone has modified that file, it would execute arbitrary JS code. Right?

What if 4chan X only stored certain attributes and their values? So, for example you could toggle viewBox or stroke or whatever, and perhaps only allow for path? That way 4chan X is only storing/saving parts of an SVG instead of the whole thing?

By then though I guess it defeats the purpose of making it easier for the user anyway. Looks like best advice is to avoid user-uploaded SVGs, because there's lots of ways around it.

If there's anything you'd like me to do with this PR let me know. I agree with you about placing another data- attribute on the parent. If people really wanted to, they could bundle their own web font in a userstyle with a @font-face dataURI and select it exactly how it is done now with FA4.7.

[ccd0]

Since the plan is to go ahead with this, shall I add your proposed wiki entry to the 4chan X wiki? (Or you could add it; the wiki should be open for anyone to edit.)

Is this article of any relevance/help?

It's relevant, but it doesn't solve the problem. I considered using SVGs in <img> tags with a data-uri source, but those don't respect currentColor.

But if someone has modified that file, it would execute arbitrary JS code. Right?

More or less. I should clarify that a website or XSS exploit is unlikely to be able to modify files directly, but if 4chan X is running on the site, it could open the settings panel and simulate user input. Cleanup would be difficult, and a well-crafted exploit could modify the user's view of the settings to hide the infection.

A notable example of an attack along these lines in the wild was the 2015 Imgur/8chan breach, which persistified itself using 8chan's setting to display a user's favorite boards in the header.
https://pastebin.com/heYvWu5Y

What if 4chan X only stored certain attributes and their values?

This would work, but I agree that it would not be user-friendly. The best UI I can imagine for this would reject many SVG files with a reason not intelligible to most users.

I agree with you about placing another data- attribute on the parent.

Good to hear. You say "another"; is there a reason to keep the data- attribute on the child?

If there's anything you'd like me to do with this PR let me know.

I think most of what's left is testing that I'll have to do myself. I'll let you know if I find any problems I don't have an immediate fix for.

[saxamaphone69]

wiki entry

I'll have another pass of the entry closer to a potential merge in case there are some changes made.

option in settings

One could argue (for the sake of arguing, not that I agree/disagree) if a user wanted to change their icons to different SVGs, first they'd have to know that it was an option in the settings, and second what an SVG is and potentially some of the features of them. If there's a note saying "you can only use x, y, and z" with a live preview of the icon on the left to see if it worked or not, then that person would be willing to confine to those restrictions.

data on child

True. If you want to target the SVG, then a[data-icon="eye"] > svg would suffice.

test

Sounds good.

[ccd0]

On the svgicons branch I've pulled in the changes from this pull request and written a little script tools/icons.js to automate the conversion process. You call it as
node tools/icons.js [names of icons to convert]
and it saves them all in src/Icons.

[saxamaphone69]

Damn, that is fancy 😮! So that's what I was trying to convey in my head, but you know, what it actually would look like. Great work!

[saxamaphone69]

I feel I should mention, if you want to change the class naming convention, by all means go for it. I must have been in more of a BEM-esque mood when I wrote them.

ninja edit Only because you have fcx-announcement, where fcx- is the prefix as opposed to what I wrote.

[ccd0]

I think the clipping to bounding boxes is messing up the intended size relationships between the icons. It's most visible with the close icon, but you can see in the header shortcut icons how they look more inconsistent than before. Trying to fit then fit the trimmed SVGs into squares is contributing to the problem; this is especially visible with the gallery icon, which looks smaller than it should compared to the others. I'm going to try setting the viewbox differently and see if I can't restore the original look.

[saxamaphone69]

That may also go back to what I’ve said in the previous issue where 4.7 icons aren’t consistent in their height/width. You’re right about the gallery icon being much bigger than nearly all the icons, which also mucks up visual consistency (plus the whole square is one size but a circle is bigger due to human eye playing tricks thing).

I’m on my phone now, but I included a Codepen either here or in the other issue which had all the icons together which showcased the inconsistency of the sizes in 4.7.

[ccd0]

I've removed the clipping and made a few other adjustments:
eba07e7

[saxamaphone69]

Oh no, what have I done...

[saxamaphone69]

I'm trying to git revert -m 1 f8c248354 or even eba07e7 but no luck. This is not what I wanted... Shit.

edit: how i did it

I wanted to try and pull/update my local version so I could test this. Not sure if using negative margins to position elements/SVGs is the best way to go about it, given that things were okay with height/width being set. I had icons at different sizes (QR icons, menu button, header icons primarily) because it visually looked the same from master to SVG, so that the average user would (/should) not have noticed any change.

[ccd0]

Just remove those last two commits (git reset d3a863f1ff03d2d3100e3315885afab857a7f4c6 --hard) then do a fast-forward merge (git merge eba07e76a3a12fbf0d7feda7824cbcb43c449572).

[ccd0]

it visually looked the same from master to SVG

It really didn't; the close icons in particular were way too large. I'll try some other stuff, though. Thinking about trying trimming but putting explicit pixel width and height on each SVG so they display at a reasonable size by default.

[ccd0]

Thinking about trying trimming but putting explicit pixel width and height on each SVG so they display at a reasonable size by default.

done in c9ab48d
(previous post, now deleted, linked the wrong commit)

[saxamaphone69]

Ok, so, another silly question. I just got it working again on my PC. I now want to use your latest changes. I do git merge 7c88ea6...? (edit: tried that, didn't work. don't want to type the wrong thing and mess things up).

In the mean time, I made this:
https://codepen.io/saxamaphone69/pen/qBOOwaZ?editors=0101
which overlays the differences between the web font (green), my version of the icons (red), and your most recent pass (blue). At the moment everything is at 32px just to highlight the visual differences. Which also means your pass of SVGs are scaled, regardless if they are 9px or 12px.

Visually, the close and angle-down icon are going to cause the most strife, and perhaps those 2 can be treated differently to the rest. I think the other icons are now corrected, and should not have to be changed to look like they were. Personally, I think all the icons being the same (as in, generic class controls the size in CSS - 13px) is the most consistent and simplest. Worrying about vertical alignment and margins seems excessive, and adds extra rules that existing userstyles would either need to cancel out or rewrite anyway.

Actually, ignore me. I don't know what I'm talking about. I see glaring issues with my method of center aligning the icons and 100% scaling them. I have a feeling the fact that a and span aren't block elements, they inherit presumed line-height, and so if you display: inline-block them and play with vertical-align, it's going to take a lot of fiddling with values to get it "just right".

[saxamaphone69]

https://developer.chrome.com/blog/new-in-chrome-105/#sanitizer-api

Dead issue/PR I know, but with regards to stripping out unwanted HTML/malicious SVG code, I suppose something like the Sanitizer API could be used?