search

tongwentang / tongwentang-extension

MIT License
137 stars 15 forks source link

Update `lang` attribute of every element #64

Closed maxmellen closed 1 year ago

maxmellen commented 1 year ago

哈囉！:wave:

This PR addresses the issue pointed out in this comment: https://github.com/tongwentang/tongwentang-extension/pull/60#issuecomment-1317504129

So I had some time to look into this, and on Wikipedia, the html elements's lang attribute is indeed updated properly, but as you can see in the screenshot below, basically every element that holds text such as the main content, but also the nav bars and menus have their own lang attributes which don't get updated currently.

I've opened a PR that updates the lang attribute on every element that already has a Chinese lang attribute → https://github.com/tongwentang/tongwentang-extension/pull/64

Let me know what you think :)

image
maxmellen commented 1 year ago

Hi @t7yang!

Have you had any time to look into this? I'm not necessarily so happy with the document.querySelectorAll call here but it does seem like the most efficient way to only get the relevant elements since I can pass an attribute selector. I wonder if that selector makes the regex check for a proper Chinese language tag here redundant. WDYT?

Or do you think it would somehow be better to move this logic into updateNodes?

Happy to update the PR according to your feedback so just let me know :)

t7yang commented 1 year ago

@maxmellen sorry for late reply. yes, I also have the same thought at first glance, but I think this is the default off feature, so probably it will not affects to many users. overall, I think querySelectorAll is better than updateNodes because we are only targeting the node with lang attribute. let me think about it tomorrow.