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

Using <em> or other font attributes in section title crashes in javascript #1137

Closed
torinfo opened this issue Jul 12, 2022 · 9 comments
Closed

Comments

@torinfo
Copy link
Collaborator

torinfo commented Jul 12, 2022

This is caused by line 1684 of application.js

$(tocName)

I don't really understand what is the purpose of this piece of code, nor do I understand why it crashes, but would be good to fix a.s.a.p.

This code is in there since 2017.

@ronm123
Copy link
Collaborator

ronm123 commented Jul 12, 2022

@torinfo that's very surprising! Not so much that causes the crash but also even just making a word in the section title bold or changing it's colour via the mini toolbar. If this has been the case for a long time how come it hasn't cropped up before!?

@torinfo
Copy link
Collaborator Author

torinfo commented Jul 12, 2022

@ronm123 I suspect it is caused by the different version of jQuery needed because of the changed version of bootstrap. In the end I did not upgrade bootstrap to the version I tried first, but installed the latest patch of the version we were using. So we could look into going back to the same version of jQuery as 3.10.

@torinfo
Copy link
Collaborator Author

torinfo commented Jul 12, 2022

@ronm123 We're using the same version as in Xot now (1.9.1 which is quite old in it self) for v3.11.

For 3.10 we used 1.8.2. And that version does not crash, but does strange things as well. For example, if you use 'Page 1 as the title, only the 1 ends up in the top menu of a page.

I think the code just tries to strip out html, but I am not entirely sure.

@torinfo
Copy link
Collaborator Author

torinfo commented Jul 14, 2022

@FayCross Do you have time to have a look at this code and explain (if you still know how the code works you created in 2017) what you are trying to accomplish with the cleaning up of tocName around line 1684 of application.js?

@FayCross
Copy link
Collaborator

@torinfo I'll have a look

@FayCross
Copy link
Collaborator

@torinfo is that better? I think that code's still needed to strip out some changes to text so the section menu doesn't look odd - there might be a better way of doing that but for now that commit stops it crashing

@torinfo
Copy link
Collaborator Author

torinfo commented Jul 15, 2022

@FayCross I'll have a look. So it's just some chages and not all. I.e. It's not an option to use $(

.append(tocName).text()?

That strips out all html

@FayCross
Copy link
Collaborator

@torinfo the code was trying to just strip out a couple of things rather than every thing - that way things like super/subscripts, font awesome icons etc. will stay. I don't remember adding this so I don't mind if it's changed to be simpler but I can see that people might want some html left in there

@torinfo
Copy link
Collaborator Author

torinfo commented Jul 15, 2022

@FayCross Ah yes. I can see that. The page titles have the same issue, and the same code (around line 525) so I fixed that in the same way.

torinfo added a commit that referenced this issue Jul 15, 2022
…ashes in javascript

 - Fixed this for page titles as well
torinfo pushed a commit that referenced this issue Jul 15, 2022
torinfo added a commit that referenced this issue Jul 15, 2022
…ashes in javascript

 - Fixed this for page titles as well
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants