atom / markdown-preview

📝 Markdown preview in Atom
MIT License
1.23k stars 357 forks source link

`Save as HTML` and `Copy as HTML` stopped working in 1.32.0 with files that have fenced code blocks #552

Open SebDeclercq opened 5 years ago

SebDeclercq commented 5 years ago

Edits by @rsese to add new details based on https://github.com/atom/markdown-preview/issues/552#issuecomment-439768345

Prerequisites

Description

The "Save As HTML" and "Copy As HTML" functionalities in the MarkDown preview doesn't work anymore if your file has a fenced code block. It seems that the problem began just after the update to ~v. 1.32.2~ 1.32.0 and the restart of Atom. https://github.com/atom/atom/pull/17985 perhaps related?

Steps to Reproduce

  1. Open a Markdown file, or create new - the file must have a fenced code block e.g.
    # Hello World
console.log();

  1. Ctrl + Shift + M
  2. Ctrl + Shift + S (or Ctrl + C for "Copy As HTML")
  3. Select file
  4. Click Save

Expected behavior:

HTML file get saved and opened in new pane in Atom.

ctrlc-c or right-clicking the preview pane and selecting Copy As HTML will copy the preview in HTML for pasting.

Actual behavior:

Files is not saved nor opened. The save as dialog opens but nothing is saved.

ctrlc-c or right-clicking the preview pane and selecting Copy As HTML doesn't do anything.

Reproduces how often:

Every time if the file has a fenced code block

Versions

Atom    : 1.32.2
Electron: 2.0.9
Chrome  : 61.0.3163.100
Node    : 8.9.3

Additional Information

Last worked in 1.31.2 so maybe https://github.com/atom/atom/pull/17985 is related?

This issue appears to be the exact replica of this old issue : https://github.com/atom/markdown-preview/issues/524

rsese commented 5 years ago

Thanks for opening a new issue :+1:

I was unable to reproduce on 1.32.2 with macOS 10.12.6. What operating system are you running? Also tried on Windows 10 but couldn't reproduce there either.

Can you also open the keybinding resolver and share a screenshot of what you see when you try to ctrl+shift+s or ctrl+c from the preview pane?

SebDeclercq commented 5 years ago

I'm on Windows 10.

Here are the screenshots :

Ctrl+Shift+S image

Ctrl-C image

Note that I also updated git-plus before restarting Atom.

Note that using the context menu with right click and select "Save As HTML" or "Copy As HTML" does not work either.

rsese commented 5 years ago

Thanks for the screenshots, they look correct so not sure why nothing's happening with those commands 🤔

And it's just those commands with Markdown Preview that's not working? E.g. does ctrl-shift-s work in the Markdown file itself rather than the preview pane to "Save As" or does that also not work?

If you temporarily reset to factory defaults, does that make any difference?

mselseth commented 5 years ago

Same issue. Unable to save html file from md file. md file created in atom. used save as from file menu, tried Packages -> markdown-to-html -> Toggle. No success. The md preview works fine. Running ubuntu 18.10. Any workaround?

mselseth commented 5 years ago

Tried a variation. From test.md file, attempted to do a save as test.html extension, got an exception, known issue reported under: https://github.com/huangjinlin/atom-markdown-to-html/issues/1

Ubuntu 18.10

If I run atom with sudo, creates the html file but is a protected file, still cannot view in Chromium browser.

emorima commented 5 years ago

Same issue, unable to save some md files to html files.

And I realized that these files unable to save are included fenced code blocks starting with ``` .

I replaced ``` to div tag, then able to save html files.

same as me?

SebDeclercq commented 5 years ago

Yep, I tried without the triple backticks and it worked as expected.

rsese commented 5 years ago

Ahhh ok, I can reproduce now with just:

# Hello World

```
console.log();
```

Looks like this stopped working in 1.32.0 and worked in ~1.32.1~1.31.2 - I've updated the issue with these details.

catscarlet commented 5 years ago

Got the same issue today.

Atom: 1.32.2 x64 Windows 7 x64

50Wliu commented 5 years ago

getNextUpdatePromise is never firing.

EDIT: Removing that line fixes saving for me but breaks two tests. https://github.com/atom/markdown-preview/commit/041cf4e3c5937f9303518d050f03df0e31c52f2d

bacc commented 5 years ago

I'm having this same issue with Atom 1.33.0 on Mac OS X 10.14.1. Can't get Save As HTML or Copy As HTML to work from the preview. I have indented code blocks in my source markdown but I do not have fenced code blocks in it. I can reproduce the issue in safe mode as well.

travissutton commented 5 years ago

I am having the issue with 1.33.0 on OS X 10.14 as well. I'm seeing a weird blocking issue in devtools when trying to make the call. I do have code blocks in my example.

[Violation] Added non-passive event listener to a scroll-blocking 'mousewheel' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952
updateEventListeners @ VM22 <embedded>:14
t.exports @ VM22 <embedded>:14
patch @ VM22 <embedded>:14
updateSync @ VM22 <embedded>:14
TextEditorComponent @ VM22 <embedded>:11
getElement @ VM22 <embedded>:11
i @ VM22 <embedded>:11
a @ VM22 <embedded>:11
(anonymous) @ VM22 <embedded>:11
(anonymous) @ VM22 <embedded>:11
t.exports @ VM22 <embedded>:14
c @ VM22 <embedded>:11
e.toHTML @ VM22 <embedded>:11
(anonymous) @ VM22 <embedded>:11
S-UP commented 5 years ago

I still see this issue in 1.34.0. This is super annoying. Indeed, it seems related to using ~~~ and/or ``` in a document.

rinpach commented 5 years ago

Using div tag can saved as html, but if I wrote

<pre>
<code>
print("Hello World")
</code>
</pre>

this code , it happened the same issue.

ludovicdeluna commented 5 years ago

Same issue for Atom v1.37.0 on Ubuntu 19.04

ShaneHoran commented 5 years ago

Specifying a language (any language) in the fenced code block is a workaround. So this

```c
  IF MODE=1 THEN
      SIPFL := TRUE;
    ELSEIF MODE=2 THEN
      NOFLT := TRUE;
```

works fine in a markdown file, but this:

```
  IF MODE=1 THEN
      SIPFL := TRUE;
    ELSEIF MODE=2 THEN
      NOFLT := TRUE;
```

fails to save as HTML. I'm using Atom 1.37.0, markdown-preview 0.159.25.

grypyrg commented 5 years ago

Still happens in v1.38.2, OSX. The workaround from @ShaneHoran works.

bdaehlie commented 5 years ago

This also happens on Ubuntu 19.04. Can confirm that the workaround from @ShaneHoran works for me too. Would be great to get this fixed, there is no error thrown at all so it takes forever to figure out what's going on when you hit this bug.

bdaehlie commented 5 years ago

In case it's helpful to know, for me this is triggered by double-tab indentation blocks, not the "three ticks" blocks. If I get rid of that the save works fine.

cdsteinkuehler commented 5 years ago

I ran into this problem today using indented code blocks (4 space indent). I have to switch to explicitly declared code blocks (using 3 ticks) AND specify a language or save-as-html silently fails.

Atom: 1.38.2 Windows x64 markdown-preview: 0.160.0

bdaehlie commented 5 years ago

I did some work to track this down. Let me preface the following by saying that I haven’t written code for a living in a while, and I don’t know JavaScript very well… Just trying to contribute to this getting fixed.

In the “markdown-preview” package, the initial relevant call stack during Save As for a markdown preview is:

lib/markdown-preview-view.js: saveAs
lib/markdown-preview-view.js: getHTML
lib/renderer.js: toHTML

The basic problem is that execution in the toHTML function never proceeds past this call:

await highlightCodeBlocks(div, grammar, convertAtomEditorToStandardElement)

Execution gets stuck here waiting for promises that are never resolved, thus the save never happens. It just silently fails.

The highlightCodeBlocks function constructs a list of promises, one for each “pre” element in the HTML. For each one, a TextEditor object is created with the “pre” contents assigned to it, along with a language designation. The idea is that this editor will be used to do syntax highlighting and then the results of that process will be extracted.

In the problematic case, the default language of “text” is used. The promise function in this case is convertAtomEditorToStandardElement. That function will need to resolve the promise.

The convertAtomEditorToStandardElement function contains this code:

if (languageMode.fullyTokenized || languageMode.tree) {
    done()
} else {
    editor.onDidTokenize(done)
}

In the problematic case, the second path is taken. Instead of resolving the promise via the done() function immediately, we’re supposed to wait for done() to be called via the onDidTokenize callback. That callback is never made, thus the promise is never resolved, thus the file save fails.

Here is a patch that works, though I am not sure whether or not it is the ideal fix. Whether it is the ideal fix or not probably depends on whether or not we should expect tokenization to enter a fullyTokenized state or fire the onDidTokenize callback.

diff --git a/lib/renderer.js b/lib/renderer.js
index ecc9139..6ed32a0 100644
--- a/lib/renderer.js
+++ b/lib/renderer.js
@@ -186,6 +186,13 @@ var highlightCodeBlocks = function (domFragment, grammar, editorCallback) {
     const fenceName =
       className != null ? className.replace(/^language-/, '') : defaultLanguage

+    // Don't proceed with tokenizing this if it's plain text.
+    // Tokenization will not complete, the promise will never be resolved
+    // and whatever depends on it will fail.
+    if (fenceName == 'text') {
+            continue;
+    }
+
     const editor = new TextEditor({
       readonly: true,
       keyboardInputEnabled: false

If we want to start chasing down the rabbit hole of whether or not the onDidTokenize callback should fire in the plain text case, we can start in the main Atom repository, here:

https://github.com/atom/atom/blob/master/src/text-editor.js

To see the fullyTokenized value and places where onDidTokenize is fired, that is done in this file:

https://github.com/atom/atom/blob/master/src/text-mate-language-mode.js

Look at markTokenizationComplete and its callers.

black-snow commented 5 years ago

Specifying a language (any language) in the fenced code block is a workaround. So this

  IF MODE=1 THEN
      SIPFL := TRUE;
    ELSEIF MODE=2 THEN
      NOFLT := TRUE;

works fine in a markdown file, but this:

  IF MODE=1 THEN
      SIPFL := TRUE;
    ELSEIF MODE=2 THEN
      NOFLT := TRUE;

fails to save as HTML. I'm using Atom 1.37.0, markdown-preview 0.159.25.

That's not true for me. I specify the language for each block and it still won't save. Might be due to some languages not being detected, though.

r0light commented 5 years ago

Specifying a language (any language) in the fenced code block is a workaround. So this

  IF MODE=1 THEN
      SIPFL := TRUE;
    ELSEIF MODE=2 THEN
      NOFLT := TRUE;

works fine in a markdown file, but this:

  IF MODE=1 THEN
      SIPFL := TRUE;
    ELSEIF MODE=2 THEN
      NOFLT := TRUE;

fails to save as HTML. I'm using Atom 1.37.0, markdown-preview 0.159.25.

That's not true for me. I specify the language for each block and it still won't save. Might be due to some languages not being detected, though.

I have the same issue and want to add some more insights:

The described workaround works when using c as a language. I have a document with java and xml code blocks for which the issue persists and i cannot save it as html. But when changing everything to c, i can suddenly save it as html again. So there seems to be an issue with different languages as well and using c is not really a workaround there.

nateGeorge commented 4 years ago

Any suggestions for a workaround? Seems to still be an issue with the latest atom.

nateGeorge commented 4 years ago

My workaround for now is to use VSCode with this plugin: https://marketplace.visualstudio.com/items?itemName=manuth.markdown-converter

fslurrehman commented 4 years ago

I have same issue . Using latest atom on windows 10. Neither copying nor saving html from markdown-preview.

michael-w-williams commented 4 years ago

Add me to the list 100% reproducible. Remove fenced code blocks and the feature works fine. Same result on Windows and MacOS.

pltcldvlpr commented 4 years ago

Same on Ubuntu 18 these days.

ccoenen commented 4 years ago

The workaround with specifying a language on a code block works for me, as long as it's not "text" Thanks to @ShaneHoran and @bdaehlie I at least was able to save something.

Jeansen commented 4 years ago

Same for me with 1.42 on Debian.

luop90 commented 4 years ago

Windows with 1.42, works when syntax highlighting c/cpp code blocks, but not with java.

zyphon7 commented 4 years ago

Using atom 1.43.0 on Windows 10.

It was breaking on: ''' code inside here ''' but it worked fine if I added anything next to the opening three backticks, i.e.:

'''nonsense code inside here '''

Actually fixed it for me.

ajh789 commented 3 years ago

Using 1.50.0 on Win 10 It didn't work with makefile syntax, e.g.: '''makefile blabla '''

ceresBakalite commented 3 years ago

Add me to the list. Still reproducible in version 0.160.2 on Windows 10. Wow, 647 days. Its been a while folks.

TylerTemp commented 3 years ago

same on Ubuntu 20, even after reset by:

sudo rm /usr/local/bin/atom
sudo rm /usr/local/bin/apm
rm -rf ~/atom
rm -rf ~/.atom
rm -rf ~/.config/Atom-Shell
sudo rm -rf /usr/local/share/atom/
sudo apt-get purge atom
sudo apt-get install atom

now, save as html works with markdown without the ``` block. Once the block is back on, it stops working.

versions:

atom version: 1.50.x x64
Electron: 5.0.13
Chrome: 73.0.3683.121
Node: v12.0.0

Please please any suggestions of fixing this, I need atom markdown because some other markdown always has issues like recongnize list nested code block etc...

ajh789 commented 3 years ago

Alternatively, I would suggest looking at markdown-preview-plus. It works stably as I can see.

rikiless commented 3 years ago

Still not working in 2021. Maybe its time to quit Atom Editor? 🤔

erchbox commented 2 years ago

Still the same on Atom v1.58.0 x64 for Windows. It is amazing that this issue on a Core package has been around for almost 3 years!!

black-snow commented 2 years ago

I feel the same but at the same time - everyone's free to file a PR, right? ;)

ZLoverty commented 2 years ago

The solution by @ShaneHoran works for me in Atom 1.58.0 on windows.