satyagraha / gfm_viewer

An Eclipse plugin providing an accurate view of GitHub Flavored Markdown (.md) files
94 stars 27 forks source link

Horribly rendered preview #67

Closed memeplex closed 10 years ago

memeplex commented 10 years ago

I'm seeing this

2014-08-07-172346_573x339_scrot

for this

2014-08-07-172405_361x276_scrot

in

Linux memeplex 3.15.5-2-ARCH #1 SMP PREEMPT Fri Jul 11 07:56:02 CEST 2014 x86_64 GNU/Linux

Version: Luna Release (4.4.0) Build id: 20140612-0600

gfm viewer fetched today from the marketplace.

satyagraha commented 10 years ago

Hmm.

We don't have a particularly large number of Linux users on here, and I am a Win7-64 user so can't directly reproduce the issue.

What you should be able to do is, context menu -> view page source -> save as, and then view in a good web browser. Then via some web browser diagnostic tool you should be able to find out the CSS rules which were applied, e.g. FireBug, Chrome tools etc.

Then you might need to install a font, or we might need to evolve the CSS rules to match what happens on GitHub. The latter is a "moving target" so we can only do our best to catch up...

Hope this helps.

memeplex commented 10 years ago

I've done that already, but the page is correctly rendered in the external browser. Also, I've tested this with webkit and mozilla renderers, but to no avail. Do you have any alternative css to test? Or maybe you could explain me how to create a custom template.

memeplex commented 10 years ago

Removing all the css from the generated file and opening it with the eclipse browser more or less fixes the issue. I would like to try:

https://github.com/sindresorhus/github-markdown-css

Should I just add the css file as an alternative style sheet or should I write a new template also?

satyagraha commented 10 years ago

In https://github.com/satyagraha/gfm_viewer/tree/master/plugin/source/code/satyagraha/gfm/support/impl you will find all the default CSS and JS file. You can download these, amend them, and then have the GFM Viewer use your versions by using the preferences page as described at https://github.com/satyagraha/gfm_viewer#configuration

You can effectively completely replace the default rendering. Don't forget to regenerate the .md.html filea and force reload in browser (which caches CSS and JS).

memeplex commented 10 years ago

Ok, thanks, I will try that.

FYI: when I set the CSS URL1 to:

https://rawgit.com/sindresorhus/github-markdown-css/gh-pages/github-markdown.css

or to (this is the same file in my local filesystem):

file:///home/carlos/github-markdown.css

I get a NPE from gfm viewer.

satyagraha commented 10 years ago

From our point of view, we'd like a CSS/JS that worked well on all platforms - for obvious reasons.

If GitHub is delivering custom CSS/JS according to the browser/platform request, then that's probably not going to be feasible for this project to emulate.

My impression is that Linux browsers typically make fairly poor decisions about what font to substitute when there is no exact match. This can be worked around by providing alternate font options in the CSS.

satyagraha commented 10 years ago

Thanks for your input. I'll take a look at the weekend. )(

satyagraha commented 10 years ago

FYI there is a later version at update site http://dl.bintray.com/satyagraha/generic/1.9.0 but this has no changes in the areas you are interested in.

memeplex commented 10 years ago

Thank you for the heads up.

I've successfully used this template:

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Strict//EN" "http://www.w3.org/TR/html4/strict.dtd">
<html>
<link rel="stylesheet" type="text/css" href="https://rawgit.com/sindresorhus/github-markdown-css/gh-
pages/github-markdown.css">
<body class="markdown-body">
@{content}
</body>
</html>

To recap:

  1. Setting a custom CSS URL throws a NPE.
  2. There is a problem in (my) linux using the provided CSS.
  3. The alternative CSS works fine.

I'm not able to help with 1 without learning how to write an eclipse plugin first but I could take a look into the CSSs in order to detect any relevant difference.

memeplex commented 10 years ago

Adding this below the body/font line does the trick:

    font-family: sans-serif;
memeplex commented 10 years ago

Regarding the NPE, here is the complete stack trace:

java.lang.NullPointerException
    at org.mvel2.templates.res.CompiledIfNode.eval(CompiledIfNode.java:43)
    at org.mvel2.templates.res.TextNode.eval(TextNode.java:42)
    at org.mvel2.templates.res.TerminalNode.eval(TerminalNode.java:35)
    at org.mvel2.templates.res.CompiledForEachNode.eval(CompiledForEachNode.java:128)
    at org.mvel2.templates.res.TextNode.eval(TextNode.java:42)
    at org.mvel2.templates.res.CompiledExpressionNode.eval(CompiledExpressionNode.java:46)
    at org.mvel2.templates.res.TextNode.eval(TextNode.java:42)
    at org.mvel2.templates.res.TextNode.eval(TextNode.java:42)
    at org.mvel2.templates.TemplateRuntime.execute(TemplateRuntime.java:285)
    at org.mvel2.templates.TemplateRuntime.execute(TemplateRuntime.java:247)
    at org.mvel2.templates.TemplateRuntime.execute(TemplateRuntime.java:255)
    at org.mvel2.templates.TemplateRuntime.execute(TemplateRuntime.java:171)
    at code.satyagraha.gfm.support.impl.TransformerDefault.transformMarkdownFile(TransformerDefault.java:90)
    at code.satyagraha.gfm.ui.impl.SchedulerDefault$2.run(SchedulerDefault.java:62)
    at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)

any idea?

satyagraha commented 10 years ago

There needs to be a minor change to the MVEL2 template in two places, so lines 9 & 14 read thus:

@if{cssText != empty}
...
@if{jsText != empty}

I tried the sindreshorus CSS, however on my system it did by default appear to suppress the scroll bars for some reason.

You can access this project's develop branch CSS/JSS at https://github.com/satyagraha/gfm_viewer/tree/develop/plugin/source/code/satyagraha/gfm/support/impl .

Hope this helps.

satyagraha commented 10 years ago

Fixed in 1.9.2.