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

Select all -> change styling (e.g. font-size) doesn't always walk the entire DOM #4471

Closed
DuncanHouston opened this issue May 17, 2023 · 7 comments

Comments

@DuncanHouston
Copy link

Description of your Issue or Request:

Certain HTML content causes walk() to return early due to nextPointWithEmptyNode() returning null.

steps to reproduce:

  1. if you paste the following HTML into the code editor:
<p> First paragraph </p><p> <br> </p><p><a href="https://www.google.com" target="_blank"> LINK </a> <br> </p><p><br></p><p> <u> UNREACHABLE </u> </p>
  1. Then select all -> font-size 20, walk() will not process the UNREACHABLE text.

The reason for this is in nextPointWithEmptyNode(): if the current node is the second last child and the next node (last child) is an empty node, as well as the next sibling for the parent being empty, then null is returned and walk() leaves early.

What is your Operating System, Browser and Version and Summernote Version you are using:

This can help find and resolve any issues, place an x inside the brackets or if relevant elaborate after each choice.

  1. Operating System:
    [ ] Microsoft Windows
    [X] Apple
    [ ] Linux
    [ ] All

  2. Browser and Version:
    [ ] Brave
    [X] Chrome (latest)
    [ ] Edge
    [ ] Firefox
    [ ] Internet Explorer
    [ ] Opera
    [ ] Safari
    [ ] Other (Specify):

  3. Summernote Version, place an x inside the brackets:
    [ ] BS3
    [X] BS4
    [ ] Lite
    [ ] All

@DuncanHouston
Copy link
Author

I've submitted a pull request #4472, which fixes this. Existing tests pass.

@DennisSuitters
Copy link
Member

Thanks just merged the PR, and ran a few different scenarios to check remove what the PR does, doesn't adversely affect other stuff. Thank you.

@MichelleKoolen
Copy link

MichelleKoolen commented Jul 4, 2023

Not sure if this is the issue that appears to be resolved, but when I am for example trying to select the whole text, and I am adjusting the spacing later, suddenly the text appears to be somehow compartmentalized, and after that you can only adjust the font per compartment, as you can see in these screenshots:

Schermopname (1001)
Schermopname (1000)
Schermopname (999)

@DennisSuitters
Copy link
Member

Did you build the project locally from the latest source? Otherwise, you won't have the merged changes. I've asked the other devs in slack about doing the 0.8.20 release (overdue). @lqez @easylogic @hackerwins

@MichelleKoolen
Copy link

MichelleKoolen commented Jul 4, 2023

I asked the team, I also tested it on the summer note website, steps:

I pasted a text from my text editor
added spacing in between with enter
Select all text
select another font

result: one compartment changes

Schermopname (1003)
Schermopname (1004)

@DennisSuitters
Copy link
Member

What team?

Compartment? I think you mean element content, let's get the terminology correct, as it can cause confusion.

The version you're testing on the Summernote website doesn't have the merged changes, which is why I specifically asked about building the project locally. I would test here, but I don't know what browser, OS or which Summernote you're using. I've just tried your steps on my local machine, and it works as expected.

I might be better to create a new issue if you've done the above so we can explore this further.

@MichelleKoolen
Copy link

So there is no issue, the summernote version we use is a couple versions behind, thank you for the information because everybody was acting like I was crazy and comparing the issues with the version on the site...

wyttime04 added a commit to iq-service-inc/react-summernote that referenced this issue Dec 8, 2023
summernote v0.8.18 dom.walkPoint 無法遍歷所有節點
summernote-patch-dom plugin 覆寫 dom.walkPoint, dom.nextPointWithEmptyNode

source: summernote/summernote#4471, summernote/summernote#4472
Signed-off-by: wyttime04 <vanessa80332@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants