Automatic preview of a diff pasted in comment textarea using WikiProcessor looks slightly different before/after clicking 'preview' button.

[repeat@…]

If a user inputs a diff file in the comment textarea using WikiProcessor, a simple diff preview will be given automatically, as in first-preview.png​ (which use Trac's diff as example).

After clicking 'preview' button, a colorful diff preview will be shown instead, as in second-preview.png​.

It will be nice if the automatic preview of a diff in WikiProcessor looks the same as clicking the preview button.

[Remy Blank]

Yes, that's a known issue. The diff WikiProcessor adds a stylesheet, but the latter isn't set during the automatic preview. We should somehow transmit the list of stylesheets together with the content, and update it in the browser.

[Jun Omae]

We can load the stylesheets with $.loadStyleSheet() on the automatic preview. The following patch adds a script tag to the response of /wiki_render. I tested with Firefox 8, Chrome 16 and IE 7. Thoughts?

t10470-load-stylesheet-on-auto-preview-r10876.diff​.

[Remy Blank]

I haven't tested the patch yet, but it looks quite elegant. I wouldn't have thought of sending the stylesheets using an embedded <script> tag.

[Jun Omae]

For 0.13dev r10883

[Jun Omae]

t10470-load-stylesheet-on-auto-preview-0.13dev-r10883.diff​ for 0.13dev: Adds load_stylesheet argument to format_to_html. If True is specified, the method generates html with <script>jQuery.loadStyleSheet(...)</script> to load the additional stylesheets.

[Remy Blank]

Note that the rendering hint in the context is passed to the formatter, so you don't need to pass it explicitly to the formatting functions.

But I'm not sure you want to make a difference between the initial rendering and the "preview" rendering. Your implementation works for the wiki page preview, but won't work for ticket previews. Can't this be made unconditional?

[osimons]

By hooking into a fundamental rendering mechanism and adding <script>...</script> tags, you also make the assumption that all rendering will be destined for web page output inside the standard Trac layout. So unless jQuery (++) are already loaded correctly, any call inserted will of course fail.

To me this just runs a risk of entangling format_to_html() call with a specific purpose. This method should be output agnostic and not make assumptions about use - even though it is through a suggested keyword argument. If anything it should be handled by interacting with the context so maybe that one day will have methods that can customize output for specific purposes.

I've never really liked the structure of the context and how we bury hints inside it in a totally non-obvious manner, and call to get_hint() in arbitrary places to be used by the code as it pleases. It just makes things very complicated, and entangles the code with needs of the Trac project only. Plugins obviously have no way of interacting with this structure. To me, any supported 'hint' should be provided by a component extension point, and likewise the handling of the given hint for a given output context should also be extendible by others. The load_stylesheet hint could then be handled differently when rendering for email, Trac layout web, or even the HTML I call to generate for various purposes through the RPC interface. At that stage we could even drop the vague 'hint' term for a more formal 'feature' structure…

One example for illustration is what to expect if you want to use format_to_html() to render HTML email notification content. It is still HTML, but just not the same. Perhaps then you'd want a completely different strategy to adding content - like for instance inlining the stylesheet instead of linking to it, or maybe ensuring that it exists as content attached to the message and references pointing to cid:<content_id> instead of a server URL.

Yes, I know that for an isolated page render I could hack something that mostly works. But it would be going against the APIs instead of working with it. I'd like for the context to handle this automatically (for instance by owning the add_*() functions and mapping them as supported features), or explicitly by low-level code calling to the context to handle some specific feature (ignoring it if it isn't supported by this context). Then the "full-page rendering context", and the "auto-preview stub context" would be wired slightly differently leading to different handling of the added stylesheet.

Sorry to complicate yet another ticket… :-)

[Remy Blank]

All valid comments. TBH, I don't see how you would replace the current context hints with extension points. There's some pretty fundamental differences, for example contexts are local to a given code path, and can be nested, whereas extension points are global to the environment. So I would be very interested in understanding that point.

[osimons]

Replying to rblank:

All valid comments. TBH, I don't see how you would replace the current context hints with extension points. There's some pretty fundamental differences, for example contexts are local to a given code path, and can be nested, whereas extension points are global to the environment. So I would be very interested in understanding that point.

Sorry, got no real details… But I'll try… :-)

1. Extension points for hints would be primarily to make them visible and part of a registry of some sort, instead of a below-the-surface implementation. So maybe the stylesheet example would provide handle_stylesheet as the implementation method for to implement in various contexts.

2. Which implementation to use would be up to the rendering context to decide. We could then create template contexts for HtmlWebPageContext, HtmlWebStubContext, HtmlEmailContext where each would implement the handle_stylesheets method in the manner most suitable for each. It could decide to override it with a custom implementation, or just call default providers to handle it - or raise an error if code calls on a feature that isn't present in current installation/version (or pass it silently if that makes more sense).

3. Plugins would be able to construct and implement other template contexts, and there is no reason why we shouldn't make it possible to override the template context selected for a given task. That means a plugin could provide a specific alternative implementation for a given situation. Perhaps we could make the generation of the initial context a pluggable system too, so that contexts are created by calling something like (pseudo):

   context = ContextManager(env).make_context('html_web_page', **kwargs)
   

   kwargs could be the regulars like perm, href and so on, but alternatively also context if we want to nest a html_web_stub inside a html_web_page.

I see your point about global/local issue, and agree: We could not make the context a component as we don't want to share structures with all that entails. So the API would need to stay at a higher level and just provide lookup and template features for generating short-lived rendering context structures.

Like current add_stylesheet tied into Chrome would really just be needed when rendering a full page, while just calling context.handle_stylesheet('my_stylesheet') would also mean that the caller (perhaps from deep inside some markup handler) would not need to worry about req and what that looks like - like knowing there would need to exist some req.chrome dict for storing stylesheets. This non-obvious structure is one of the reasons why it is so hard to things like #2625 properly, and also ties in with the ongoing discussion at #10125 that also execute outside regular request context. For #2625 I have HTML email working for my own needs, but apart from a few predictable fields I don't dare to render HTML markup for something like ticket comments as user content can call on any wiki processor that may in turn make any assumption about req directly or indirectly that I would never be able to predict in my mock request object.

I'll be the first to admit that the formatter/rendering/context code is an area where I've spent little time. I've always found it non-obvious and hard to understand, so I'm quite sure there are essential aspects that I've missed. However, particularly with the RPC plugin I'm probably the one that most often bump my head against issues and assumptions made by buried conditionals deep inside code that I cannot influence. I just want formatting and contexts to be less entangled. I don't know exactly how… :-)

[Christian Boos]

Replying to osimons:

(snip) For #2625 I have HTML email working for my own needs, but apart from a few predictable fields I don't dare to render HTML markup for something like ticket comments as user content can call on any wiki processor that may in turn make any assumption about req directly or indirectly that I would never be able to predict in my mock request object.

The main idea behind the RenderingContext was precisely to get progressively rid of the (web) Request as the central part of the rendering, as you're not always rendering HTML and even for HTML, not always for a browser. The rendering context "knows" how things should be rendered, and you can focus on providing the content. Now, that the API could be improved and that the hints could evolve to something more structured with pluggable behavior, that's certainly possible and desirable. Material for post-1.0 ;-)

(snip) However, particularly with the RPC plugin I'm probably the one that most often bump my head against issues and assumptions made by buried conditionals deep inside code that I cannot influence.

Another wish for post-1.0, maybe it will be a good idea to integrate the ​TH:XmlRpcPlugin into core, as this will provide a few opportunities to refactor the code, as often discussed.

[Christian Boos]

One comment about the patch t10470-load-stylesheet-on-auto-preview-0.13dev-r10883.diff​ itself: I haven't studied it in details yet, but I don't think that each wiki_to_html should have to care about adding a load_stylesheet parameter. We should find a way which makes it implicit.

Also moving to 1.1.x for the points discussed in comment:9 and above.

[Jun Omae]

New proposed changes in [98eebd9c/jomae.git] (jomae.git@t10470_1.0) and [acd4654e/jomae.git] (jomae.git@t10470_0.12). I've created these branches on Jan 2014 and rebased now.

The changes added jQuery.loadStyleSheet(...) to content sending from trac/wiki/web_api.py and trac/ticket/web_ui.py using filter_stream.

For 0.12.x users, using ​th:TracFewFixesPlugin is able to fix it.

[Jun Omae]

Replying to jomae:

New proposed changes in [98eebd9c/jomae.git] (jomae.git@t10470_1.0) and [acd4654e/jomae.git] (jomae.git@t10470_0.12). I've created these branches on Jan 2014 and rebased now.

The changes added jQuery.loadStyleSheet(...) to content sending from trac/wiki/web_api.py and trac/ticket/web_ui.py using filter_stream.

Updated jomae.git@t10470_1.0. filter_stream isn't used in the changes.

[Jun Omae]

I'm retrying to fix it, jomae.git@t10470_1.0. After the changes, XHR response of auto-preview includes script elements with jQuery.loadStylesheet(). Could someone please review the changes?

[Christian Boos]

Reviewing:

• in source:jomae.git/trac/wiki/web_api.py@aad5c02d​, there's a new from genshi.builder import tag which doesn't seem to be used
• chrome_info_script could return '' (simplifying the calls where it's followed by the or ''); otherwise if there's a real need to have None we could also simplify by leaving out the else: None

[Jun Omae]

Thanks for the reviewing. Committed based on the review in [13328] and merged to trunk in [13329].

[lkraav <leho@…>]

I had to revert this ​28b914a or [13329] in one of my production branches, where site.html doesn't load trac.css at all because it's using its own CSS on a blank canvas.

Now trac.css gets shoved back in during preview and ruins everything. How do I filter this out?

[Jun Omae]

Now trac.css gets shoved back in during preview and ruins everything. How do I filter this out?

Currently, It is not able to filter contents of the auto-preview by site.html. Similar to #11199.

Instead, I think that you can use the following small plugin in your case. If the plugin is installed to your $TRACENV/plugins, it sends empty contents when trac.css is requested.

# -*- coding: utf-8 -*-

from trac.core import Component, implements
from trac.web.api import IRequestFilter
from trac.web.chrome import Chrome

class AvoidTracCss(Component):

    implements(IRequestFilter)

    def pre_process_request(self, req, handler):
        if isinstance(handler, Chrome) and \
                req.args['prefix'] == 'common' and \
                req.args['filename'] == 'css/trac.css':
            return self
        return handler

    def post_process_request(self, req, template, data, content_type):
        return template, data, content_type

    def process_request(self, req):
        return req.send('', 'text/css')

[lkraav <leho@…>]

Replying to jomae:

Instead, I think that you can use the following small plugin in your case. If the plugin is installed to your $TRACENV/plugins, it sends empty contents when trac.css is requested.

Thanks @jomae, but the plugin as written doesn't kill off the injection. Haven't had time to debug specifics yet. debug.log shows the plugin does get loaded, but I think it's missing something.

[Jun Omae]

Replying to lkraav <leho@…>:

Thanks @jomae, but the plugin as written doesn't kill off the injection. Haven't had time to debug specifics yet. debug.log shows the plugin does get loaded, but I think it's missing something.

Confirm empty response for trac.css using like this and try force-reload.

wget -S -O - http://your.trac.site/trac/chrome/common/css/trac.css