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

quotations marks inside HTML attributes aren't escaped properly when merging #1462

Closed
MadaraUchiha opened this issue Feb 24, 2017 · 7 comments · Fixed by #2235
Closed

quotations marks inside HTML attributes aren't escaped properly when merging #1462

MadaraUchiha opened this issue Feb 24, 2017 · 7 comments · Fixed by #2235
Labels
good first issue Should be easier for first time contributors help welcome Could use help from community parser
Milestone

Comments

@MadaraUchiha
Copy link

I am using Highlight.js 9.0.0 which comes by default with reveal.js. Browser is chrome latest stable.

Given the following

<pre><code data-trim data-noescape class="typescript fragment">
// type Pick&lt;T, K extends keyof T> = {
//     [P in K]: T[P];
// }
function pick&lt;T, K extends keyof T>(obj: T, keys: K[]): Pick&lt;T, K> {
    // ...
}

const employee = {name: 'Madara', age: 125, profession: 'Developer'};
const person = pick(employee, ['name', 'age']); // {name: string, age: number}
const oops = pick(employee, <span data-title=" Type '&quot;height&quot;' is not assignable to type '&quot;name&quot; | &quot;age'&quot; | &quot;profession&quot;'.">['name', 'height']</span>)
</code></pre>

I see "Type '" in the title, looking at the DOM through the devtools it looks like highlight.js unescapes the attribute and possible inserts unescaped entities as innerHTML, the resulting DOM looks like this:

<!-- snip -->
<span data-title=" Type '"height"' .... ">...
<!-- /snip -->

Which causes the data-title attribute to end prematurely.

@Zemnmez
Copy link

Zemnmez commented Apr 8, 2019

the snippet you have here has the part you're saying got unescaped by hl.js already unescaped. You can see it in github's own highlight.

@joshgoebel
Copy link
Member

joshgoebel commented Oct 6, 2019

@MadaraUchiha If this is still an issue could you make a https://jsfiddle.net showing the exactly issue that way we can make sure we're all talking on the same page?

@joshgoebel joshgoebel added the needs reproduction Needs a reproducible example or sample label Oct 7, 2019
@joshgoebel
Copy link
Member

Ping.

@MadaraUchiha
Copy link
Author

MadaraUchiha commented Oct 14, 2019

@yyyc514 Apologies, here is the reproduction case: https://jsfiddle.net/Ltrb1e8q/

See how in the HTML source, the data-title attribute is complete, but in the result DOM, it is sliced off.

Note that I did not try to reproduce this with the newer versions of Highlight.js, I used the version I reported originally.

@joshgoebel
Copy link
Member

Note that I did not try to reproduce this with the newer versions of Highlight.js, I used the version I reported originally.

That's not helpful. I believe you that it was an issue THEN. :-) We need to know if it's still an active issue or not.

You can fork my jsfiddle if it helps:
https://jsfiddle.net/ajoshguy/a1kntyg4/

Thanks.

@joshgoebel
Copy link
Member

Reproduction:
https://jsfiddle.net/ajoshguy/j8vhbr03/3/

Pretty sure issue is:

function attr_str(a) {return ' ' + a.nodeName + '="' + escape(a.value).replace('"', '&quot;') + '"';}

Replace only does the replace a single time, when we need to replace ALL the quotes with &quot; again.

@joshgoebel joshgoebel added good first issue Should be easier for first time contributors help welcome Could use help from community parser and removed needs reproduction Needs a reproducible example or sample labels Oct 24, 2019
@joshgoebel joshgoebel added this to the 9.15.12 milestone Oct 24, 2019
@joshgoebel joshgoebel changed the title Highlight.js unescapes HTML entities in attributes when in "don't escape" mode. quotations marks inside HTML attributes aren't escaped properly when merging Oct 24, 2019
@joshgoebel
Copy link
Member

No idea about data-trim data-noescape though, those are Reveal features perhaps? But this is an actual bug so we can fix the actual bug in core and hopefully that'll help you out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Should be easier for first time contributors help welcome Could use help from community parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants