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

duplicate bookmark will break include #1393

Closed
ezzra opened this issue Apr 15, 2019 · 4 comments

Comments

2 participants
@ezzra
Copy link

commented Apr 15, 2019

When using copy&paste for texts and lists, it might happens that there are duplicate bookmarks on one page. When including anything from that page, this will produce a fatal error on the page with include template:

production.ERROR: DOMDocument::loadHTML(): ID bkmrk-xxx-%28 already defined in Entity, line: 45 {"userId":x,"email":"xx@xx","exception":"[object] (ErrorException(code: 0): DOMDocument::loadHTML(): ID bkmrk-xxx-%28 already defined in Entity, line: 45 at /var/www/xx/bookstack/app/Entities/Repos/EntityRepo.php:664)

Reproduce
Create a page A with duplicate bookmarks, like <ul id="bkmrk-xxx-%28"></ul> <ul id="bkmrk-xxx-%28"></ul>. This page will still work. But if you include something from that page within page B, B will return an error.

@ssddanbrown ssddanbrown added this to the v0.26.0 milestone Apr 15, 2019

ssddanbrown added a commit that referenced this issue Apr 15, 2019

@ssddanbrown

This comment has been minimized.

Copy link
Member

commented Apr 15, 2019

Thanks for reporting @ezzra, Can confirm the issue.
I have made an update in c380c10, along with a test to cover, to prevent the page hard-failing and instead still render but without the included content.

I'll keep this open as it'll be good to update the logic on save to de-dupe any ids.

@ssddanbrown

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

Hi @ezzra,
Can you confirm the editor and/or process you're following to save the content with duplicate ID's?

Just been digging into the content saving logic and found there's already code in place to de-dupe bkmrk ids. Having exact steps/content for replicating this would really help.

ssddanbrown added a commit that referenced this issue Apr 20, 2019

@ezzra

This comment has been minimized.

Copy link
Author

commented Apr 20, 2019

It looks like it can happen with nested lists:


<ul id="bkmrk-eins-zwei-drei">
<li>eins</li>
<li>zwei</li>
<li>drei</li>
<ul id="bkmrk-eins-zwei-drei">
<li>eins</li>
<li>zwei</li>
<li>drei</li>
</ul>
</ul>

Code like that was a result from copy&pasting I think.

@ssddanbrown

This comment has been minimized.

Copy link
Member

commented Apr 21, 2019

Thanks for confirming, Could now replicate. I've added logic to de-dupe ID's on all child elements to prevent such occurrences persisting after saving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.