sciencehistory / chf-sufia

sufia-based hydra app
Other
9 stars 4 forks source link

Adding a target=_blank to description links #1205

Closed eddierubeiz closed 5 years ago

eddierubeiz commented 5 years ago

Uses nokogiri to add in the new link attribute. Note that the target=_blank is not stored, but added at display time; the tests reflect this. Fixes #1183

jrochkind commented 5 years ago

Looks good.

It's a bit unfortunate that I think this parse with Nokogiri is probably a second parse with Nokogiri, I think the loofah based sanitizer also does a nokogiri parse. But there is probably no great way to hook into it to do something different than loofah is doing. This is probably good enough.

jrochkind commented 5 years ago

Hmm, hypothetically we could possibly write a custom loofah scrubber to hook into the single nokogiri parse.

https://github.com/flavorjones/loofah#the-basics

I'm not sure what that code would look like, or how much of what's already there would have to be changed. Do you think it's worth spending more time on?

jrochkind commented 5 years ago

@eddierubeiz I did it and pushed it as an additional commit. I'm doubting myself lately, not sure if this is an improvement, the opposite, or just shuffling deck chairs. Feedback welcome.