@NeatNit commented on this pull request.
> + # "li", # child; sometimes container
+
+ "table", # similar to Discourse, where the <div> parent of <table>
gets dir="auto" (no parent div in kramdown)
+ # "td", # child
+ # "th", # child; not actually a thing in kramdown but hypothetically
might be in the future
+
+ "dl",
+ # "dd", "dt", # child - since it's similar to a list, it has the same
behavior as a list
+
+ "math" # don't know how this ends up, but dir="auto" is probably
correct
+ ].each do |name|
+ define_method :"convert_#{name}" do |el, indent|
+ attr_bak = el.attr.dup # can't avoid mutating the attr hash, so make
a backup
+ el.attr["dir"] = "auto" unless el.attr.key?("dir") # if by some
miracle dir is already defined, don't override it
+ ret = super(el, indent)
+ el.attr.replace(attr_bak) # restore backup
> What happens if we don't restore the original attributes? Does the element
> get reused in a way that breaks things?
Truthfully, nothing. It's only a concern if you ever use a converter other than
HtmlBidi. Since we don't, it doesn't matter, and we can mutate the Document
freely.
> would just doing `el = el.dup` as the first thing to duplicate the element be
> a workable alternative?
No, because it would still point to the same `attr` hash, and mutating it would
mutate the original. But as I said, it doesn't matter, so the whole
backup-restore code can be removed if you wish.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5840#discussion_r2014856583
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/5840/review/2718388...@github.com>_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev
