Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Component/s: moodle.org
    • Labels:
      None

      Description

      see attchments to understand this issue
      or browse the page http://moodle.org/mod/forum/discuss.php?d=149803

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            tsala Helen Foster added a comment -

            Daniele, thanks for your report with screenshot illustrating the problem. Assigning to Sam.

            Show
            tsala Helen Foster added a comment - Daniele, thanks for your report with screenshot illustrating the problem. Assigning to Sam.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks for the report Daniele.
            I've confirmed the problem, it is being caused by a clash between autolinking and the syntax highlighting.
            I'll start looking into this shortly.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks for the report Daniele. I've confirmed the problem, it is being caused by a clash between autolinking and the syntax highlighting. I'll start looking into this shortly. Cheers Sam
            Hide
            mudrd8mz David Mudrak added a comment -

            Sam is right. The problem is there because of the current text filtering logic:

            1. Forum module calls format_text() when displaying the post message
            2. The message text is converted into HTML by s() or markdown_to_html() or text_to_html() according the text format used for the post
            3. The message text in HTML format is cleaned up by clean_text()
            4. The chain of filters is applied to the message text

            Now, when the post is in FORMAT_MOODLE (as in the case of attached example), text_to_html() is called in step 2. This function calls convert_urls_into_links() and so GeSHi filter has no chance to get to the original text without <a>

            From my point of view, there is the only way how to get around this: get rid of convert_urls_into_links(), move it from core and transform it to a normal filter. Then admin would just set it as the last element in the filter chain and GeSHi would work correctly (tested on my own machine).

            There is an issue, though. Currently, filters are not informed about the format in which the text was originally written - because at the moment they can be applied, the text is in HTML only. Therefore the new filter_urltolink would apply to all other formats, too. That breaks backwards compatibility as only URLs in FORMAT_MOODLE texts are converted now. But it is not that hard to extend our current filters API and provide them the information of the format.

            Please note that the same logic should apply to replace_smilies() - it should be converted into ordinary filter, too.

            I'll be happy to work on this. Should not take more than one day as the code and unit tests are already written.

            Show
            mudrd8mz David Mudrak added a comment - Sam is right. The problem is there because of the current text filtering logic: 1. Forum module calls format_text() when displaying the post message 2. The message text is converted into HTML by s() or markdown_to_html() or text_to_html() according the text format used for the post 3. The message text in HTML format is cleaned up by clean_text() 4. The chain of filters is applied to the message text Now, when the post is in FORMAT_MOODLE (as in the case of attached example), text_to_html() is called in step 2. This function calls convert_urls_into_links() and so GeSHi filter has no chance to get to the original text without <a> From my point of view, there is the only way how to get around this: get rid of convert_urls_into_links(), move it from core and transform it to a normal filter. Then admin would just set it as the last element in the filter chain and GeSHi would work correctly (tested on my own machine). There is an issue, though. Currently, filters are not informed about the format in which the text was originally written - because at the moment they can be applied, the text is in HTML only. Therefore the new filter_urltolink would apply to all other formats, too. That breaks backwards compatibility as only URLs in FORMAT_MOODLE texts are converted now. But it is not that hard to extend our current filters API and provide them the information of the format. Please note that the same logic should apply to replace_smilies() - it should be converted into ordinary filter, too. I'll be happy to work on this. Should not take more than one day as the code and unit tests are already written.
            Hide
            dougiamas Martin Dougiamas added a comment -

            This makes sense to me. +1 for that

            BUT how do you propose to extend the filter API?

            We need some kind of named parameters (eg object passed) because I think there were other similar problems needing solving too.

            I'd also like to see some backward compatibility for old filters.

            Show
            dougiamas Martin Dougiamas added a comment - This makes sense to me. +1 for that BUT how do you propose to extend the filter API? We need some kind of named parameters (eg object passed) because I think there were other similar problems needing solving too. I'd also like to see some backward compatibility for old filters.
            Hide
            mudrd8mz David Mudrak added a comment -

            Martin, the proposal to extend the filter_manager API is tracked in MDL-24531 together with a patch illustrating it. If we do not extend the API (so that the original format is passed to the filter), the new filter_urltolink would have to be applied to all formats and that would break BC (because currently it converts URLs only in FORMAT_MOODLE). Therefore I set MDL-24531 as the blocker for this issue. If the patch proposed in the blocker is accepted, I have working fix for this issue too.

            Show
            mudrd8mz David Mudrak added a comment - Martin, the proposal to extend the filter_manager API is tracked in MDL-24531 together with a patch illustrating it. If we do not extend the API (so that the original format is passed to the filter), the new filter_urltolink would have to be applied to all formats and that would break BC (because currently it converts URLs only in FORMAT_MOODLE). Therefore I set MDL-24531 as the blocker for this issue. If the patch proposed in the blocker is accepted, I have working fix for this issue too.
            Hide
            mudrd8mz David Mudrak added a comment -

            Hi Sam,

            this should be now solved. I just committed new filter urltolink. At moodle.org, please activate that filter and make sure that it is applied after the GeSHi filter (it makes sense to me to have urltolink one of the last filters in the chain).

            Show
            mudrd8mz David Mudrak added a comment - Hi Sam, this should be now solved. I just committed new filter urltolink. At moodle.org, please activate that filter and make sure that it is applied after the GeSHi filter (it makes sense to me to have urltolink one of the last filters in the chain).

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Development