Bug 1520004: Update Prism to version 1.15 #5189
Conversation
ExE-Boss
force-pushed
the
libs/prism/bug1520004
branch
from
90d4962
to
d68ee9c
Compare
|
Sorry for the noise. I had to close and open the PR because our TravisCI integration was broken when this was opened. |
|
Why has the flake8 job failed? |
|
TravisCI hasn't been building for a while. A PR with that error was merged to master, and your commit was based on it. We can ignore it or you can rebase on current master to get rid of it. |
|
Let’s ignore it and merge this. |
ExE-Boss
changed the title
ExE-Boss
changed the title
|
This has no conflicts with #5182 if that’s what you’re worried about. |
|
@ExE-Boss I appreciate you thinking of me, but I have nothing to contribute here. |
|
I meant to write @jwhitlock |
I wrote a 900 word response, but I think it belongs on Discourse instead of a PR comment. There is no code conflict, but there are conflicts:
I'm assigning @schalkneethling as reviewer, but it is up to him if reviewing this is compatible with his other priorities. |
|
We've shipped the KumaScript refactor, but the other blockers remain. I don't think there is much that can unblock the asset pipeline work, other than not taking time for other reviews. For the Prism changes, it would be helpful to link to the Prism changelog, if available, and see if there are any warnings for this upgrade path. For the content changes, it would be helpful to find the URLs of pages that would be affected by this change. Screenshots of the before and after would be super helpful. If there are no visual changes, it would be good to know if that is success (nothing broke!) or evidence that we don't need the update. |
|
Here’s the Prism changelog: |
|
@ExE-Boss is there a specific feature or improvement to Primsjs that makes you feel like this is a priority for us to update? Might there be a performance hit if we add support for more programming languages? This touches almost all pages on MDN Web Docs, so I just want to be very cautious when upgrading. We are pretty far behind, and these are dot releases but, pays to be on the safe side though ;) Let me know your thoughts, thanks! |
|
The primary reason is that JSON syntax highlighting is now built into Prism, which means that it no longer extends the JavaScript syntax and isn’t as error prone as the current JSON syntax. As for performance, there shouldn’t be any noticeable difference as the number of Prism language syntax definitions installed on MDN stays the same. In fact, performance should be improved, as there have been a lot of optimisations to the regexes to make them faster. Also, Prism 1.14 added support for textual WebAssembly, which might be useful to add in a follow‑up. |
|
I’ve now added a section to my Prism issues page on MDN for a comparison of the current JSON syntax definition and the official Prism JSON syntax definition: |
|
I have now opened bug 1527533 to add the PrismJS syntax definition for the WebAssembly textual representation. |
jwhitlock
requested review from
jwhitlock
and removed request for
schalkneethling
|
I'm going to prepare this PR for merging today. |
* prism 1.2.0 → 1.15: 3 years of updates. Some XSS mitigation (1.6.0), drop IE8 support, regex optimization and other syntax updates and additions. JSON was added in 1.4.0, so switch from custom syntax component to Prism's. I ran bower-installer to update the prism package as well as the syntax plugins and CSS
jwhitlock
force-pushed
the
libs/prism/bug1520004
branch
from
d68ee9c
to
1ab14d1
Compare
|
Force-push with more packages updated from |
…5269) Fixes [bug 1529886](https://bugzilla.mozilla.org/show_bug.cgi?id=1529886) See also #5189
Resolves bug 1520004.
Extracted from #5157.
review?(@escattone, @jwhitlock)