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

compatibility issue with chrome highlight extension #199

Open
sapjax opened this issue Nov 30, 2022 · 1 comment
Open

compatibility issue with chrome highlight extension #199

sapjax opened this issue Nov 30, 2022 · 1 comment
Labels
enhancement New feature or request

Comments

@sapjax
Copy link

sapjax commented Nov 30, 2022

I'm currently developing a chrome extension, which highlights some English words in the text, there is a compatibility issue.

the highlight function replaces the word with an HTML tag, for example, a sentence like:

<p>Imgbot is a friendly robot that optimizes your images and saves you time.</p>

may be replaced by

<p>Imgbot is a friendly robot that <mark>optimizes</mark> your images and saves you time.</p>

The problem is that the ebook-reader's code keeps the text nodes reference in the js memory:

and use the references later:

but after the extension replaced the text, the paragraph text node does not exist anymore, so when it runs into range.selectNode(node); will throw out an error, and the page renders nothing.

I think we can make some changes here to avoid such compatibility issues with extension.

  • do not keep the references, and query the paragraphs every time.
  • use the parent P tag instead of using the text node inside.

what do you think?

@Renji-XD
Copy link
Collaborator

Renji-XD commented Jan 9, 2023

Hi,

  • i don't think that requerying in this method would help as it is just executed in case of initial load/resizing or switchting between chapters in pagination. Any frequent requerying is maybe also overkill / unecessary for people not using your extension
  • main audience of the reader are people reading japanese books. these often contain furigana which will create additional text nodes and therefore more frequent update to the character progress - therefore i guess the preference is to keep it on this level

In context of #21 there will probably some changes introduced - we will try to keep this one in mind for it (though i don't know when it may will be taken up) and therefore i will leave it open

@Renji-XD Renji-XD added documentation Improvements or additions to documentation enhancement New feature or request and removed documentation Improvements or additions to documentation labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants