Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Filters
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      1) OOP formats, making them pluggable (this is apart but could be good idea for 2.0)
      2) OOP filters
      3) Allow each filter to alter the md5key generated, via filter method. filter->hash() will return a unique string, the results for this could be something like, in this example: "censor:seecensoredwords" for users having the capability, or: "censor:" for the rest. Note that adding this info to the hash sounds nice because it automatically invalidates records when filters are changed, and that isn't happening right now.
      4) Make the rest of format_text() and format_string() to work as now, but using that "custom" $md5key that contains all the particularities of the text being formatted.

      This improvement will help fix this bug:
      > Students at our school figured out they could swear if they put the word within a hyperlink - e.g.
      > <a href='#'>SwearWord</a>

        Gliffy Diagrams

        1. filter2.patch
          52 kB
          Dongsheng Cai
        2. MDL-14582_filter_plugins.patch
          61 kB
          Dongsheng Cai
        3. MDL-14582_filters.patch
          51 kB
          Dongsheng Cai
        4. MDL-14582_lib.patch
          5 kB
          Dongsheng Cai
        5. MDL-14582_weblib_filterlib.patch
          6 kB
          Dongsheng Cai
        6. Moodle Censorship Filter Modification.pdf
          87 kB
          guy thomas

          Issue Links

            Activity

            Hide
            dongsheng Dongsheng Cai added a comment -

            Put a patch here for further discuss.

            1. isteacher is a deprecated functions, so in my patch, I use has_capability('moodle/site:doanything', $context) instead
            2. I use return filter_phrases($text, $words, true); instead of a <dummytag>, cause your students might swear using <dummytag>swear<dummytag>

            Please review

            Show
            dongsheng Dongsheng Cai added a comment - Put a patch here for further discuss. 1. isteacher is a deprecated functions, so in my patch, I use has_capability('moodle/site:doanything', $context) instead 2. I use return filter_phrases($text, $words, true); instead of a <dummytag>, cause your students might swear using <dummytag>swear<dummytag> Please review
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Some comments:

            1) Look the get_context_instance(CONTEXT_SYSTEM, SITEID) as commented.
            2) I like the approach of hiding the original to all but admins, but...
            3) Require MD approval and fixfor versions (I think we could run it for some time in HEAD in our servers and then backport to 1.9 and 1.8).

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Some comments: 1) Look the get_context_instance(CONTEXT_SYSTEM, SITEID) as commented. 2) I like the approach of hiding the original to all but admins, but... 3) Require MD approval and fixfor versions (I think we could run it for some time in HEAD in our servers and then backport to 1.9 and 1.8).
            Hide
            dongsheng Dongsheng Cai added a comment -

            Journal mod has been moved to contrib.

            Show
            dongsheng Dongsheng Cai added a comment - Journal mod has been moved to contrib.
            Hide
            dongsheng Dongsheng Cai added a comment -

            I incorrectly closed this issue, reopen it.

            Show
            dongsheng Dongsheng Cai added a comment - I incorrectly closed this issue, reopen it.
            Hide
            dougiamas Martin Dougiamas added a comment -

            Note that filtered text gets cached so we shouldn't use capabilities at all ... it's possible to prevent caching when this filter is used but do we really want that sort of a performance hit (all texts processed again for every user every time)?

            If we do then it shouldn't rely on doanything, but some special capability like

            filter/censor:seecensoredwords

            Show
            dougiamas Martin Dougiamas added a comment - Note that filtered text gets cached so we shouldn't use capabilities at all ... it's possible to prevent caching when this filter is used but do we really want that sort of a performance hit (all texts processed again for every user every time)? If we do then it shouldn't rely on doanything, but some special capability like filter/censor:seecensoredwords
            Hide
            brudinie guy thomas added a comment -

            Forgot about the caching side of things.

            One possible resolution to this issue is to cache twice - one cache with censored text and one for people with the capability to read uncensored swears.
            When the page is being rendered the system would decide which cache to use according to users capabilities.

            Another possible solution is to create a new filtering mechanism which is pre-write to the db.
            Using this method, the text would be written to two tables if a swear was used.

            I.e.

            moodle_messages gets the censored text

            a new table

            moodle_prefilter gets the text prior to any filtering taking place

            Obviously I've just scratched the surface with this pre-write filter idea but I can expand on it if you think it is a viable solution.

            Show
            brudinie guy thomas added a comment - Forgot about the caching side of things. One possible resolution to this issue is to cache twice - one cache with censored text and one for people with the capability to read uncensored swears. When the page is being rendered the system would decide which cache to use according to users capabilities. Another possible solution is to create a new filtering mechanism which is pre-write to the db. Using this method, the text would be written to two tables if a swear was used. I.e. moodle_messages gets the censored text a new table moodle_prefilter gets the text prior to any filtering taking place Obviously I've just scratched the surface with this pre-write filter idea but I can expand on it if you think it is a viable solution.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Well,

            perhaps this is a matter, for 2.0 to do something like this (to bring this and similar issues fixed for ever):

            1) OOP formats, making them pluggable (this is apart but could be good idea for 2.0)
            2) OOP filters
            3) Allow each filter to alter the md5key generated, via filter method. filter->gethash(), the results for this could be something like, in this example: "censor:seecensoredwords" for users having the capability, or: "censor:" for the rest. Note that adding this info to the hash sounds nice because it automatically invalidates records when filters are changed, and that isn't happening right now.
            4) Make the rest of format_text() and format_string() to work as now, but using that "custom" $md5key that contains all the particularities of the text being formatted.

            Note that right now, for example, we are adding manually the current_language() to the cache, because of the lang filter. This will fit in the schema commented above automatically.

            Guy, I'm not really sure if the "intermediate" cache (before filtering) that you comment is really a good idea or no. Perhaps you can try to perform some basic implementation and measure differences? I guess we'll lost more time reading and witting it than the time needed to format the texts, but... who knows.

            Dong, how do you see the proposal? IMO could be a nice thing to change (one more) for Moodle 2.0. Comment it with MD...ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Well, perhaps this is a matter, for 2.0 to do something like this (to bring this and similar issues fixed for ever): 1) OOP formats, making them pluggable (this is apart but could be good idea for 2.0) 2) OOP filters 3) Allow each filter to alter the md5key generated, via filter method. filter->gethash(), the results for this could be something like, in this example: "censor:seecensoredwords" for users having the capability, or: "censor:" for the rest. Note that adding this info to the hash sounds nice because it automatically invalidates records when filters are changed, and that isn't happening right now. 4) Make the rest of format_text() and format_string() to work as now, but using that "custom" $md5key that contains all the particularities of the text being formatted. Note that right now, for example, we are adding manually the current_language() to the cache, because of the lang filter. This will fit in the schema commented above automatically. Guy, I'm not really sure if the "intermediate" cache (before filtering) that you comment is really a good idea or no. Perhaps you can try to perform some basic implementation and measure differences? I guess we'll lost more time reading and witting it than the time needed to format the texts, but... who knows. Dong, how do you see the proposal? IMO could be a nice thing to change (one more) for Moodle 2.0. Comment it with MD...ciao
            Hide
            dongsheng Dongsheng Cai added a comment -

            Sounds good, I will start to work on this issue after 1.9.1 release

            Show
            dongsheng Dongsheng Cai added a comment - Sounds good, I will start to work on this issue after 1.9.1 release
            Hide
            dougiamas Martin Dougiamas added a comment -

            Alternatively, just admit that blanket censorship of people's words through technical means is unworkable in the long run and to just put more effort into doing it socially. Set up some rules of conduct and introduce penalties for breaking them. Reducing student grades is an obvious one but I'm sure you can get creative.

            Show
            dougiamas Martin Dougiamas added a comment - Alternatively, just admit that blanket censorship of people's words through technical means is unworkable in the long run and to just put more effort into doing it socially. Set up some rules of conduct and introduce penalties for breaking them. Reducing student grades is an obvious one but I'm sure you can get creative.
            Hide
            dougiamas Martin Dougiamas added a comment -

            But I do like Eloy's idea in general of letting all filters modify the hash

            Show
            dougiamas Martin Dougiamas added a comment - But I do like Eloy's idea in general of letting all filters modify the hash
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            grrr. sorry by the delay. I'll review this tomorrow (when I receive this mail). Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - grrr. sorry by the delay. I'll review this tomorrow (when I receive this mail). Ciao
            Hide
            dongsheng Dongsheng Cai added a comment -

            A new method is added in the abstract class

            Show
            dongsheng Dongsheng Cai added a comment - A new method is added in the abstract class
            Hide
            dongsheng Dongsheng Cai added a comment -

            Replace all the filter functions with OOP Filters.

            Show
            dongsheng Dongsheng Cai added a comment - Replace all the filter functions with OOP Filters.
            Hide
            dongsheng Dongsheng Cai added a comment -

            The changes to weblib.php and filterlib.php.

            Show
            dongsheng Dongsheng Cai added a comment - The changes to weblib.php and filterlib.php.
            Hide
            dongsheng Dongsheng Cai added a comment -

            Please review, TIA

            Show
            dongsheng Dongsheng Cai added a comment - Please review, TIA
            Hide
            dongsheng Dongsheng Cai added a comment -

            Because of $DB conversion, the old patch is not valid any more, I attach two new patch here, Eloy, can you have a look?

            Show
            dongsheng Dongsheng Cai added a comment - Because of $DB conversion, the old patch is not valid any more, I attach two new patch here, Eloy, can you have a look?
            Hide
            dougiamas Martin Dougiamas added a comment -

            Changing name of this bug ...

            Show
            dougiamas Martin Dougiamas added a comment - Changing name of this bug ...
            Hide
            dongsheng Dongsheng Cai added a comment -

            New patches against the lastest HEAD.

            Show
            dongsheng Dongsheng Cai added a comment - New patches against the lastest HEAD.
            Hide
            dongsheng Dongsheng Cai added a comment -

            This new patch provide back-compatible with old filter plugins.

            Show
            dongsheng Dongsheng Cai added a comment - This new patch provide back-compatible with old filter plugins.
            Hide
            dongsheng Dongsheng Cai added a comment -

            resolve it for testing, feel free to reopen it
            Thanks

            Show
            dongsheng Dongsheng Cai added a comment - resolve it for testing, feel free to reopen it Thanks
            Hide
            timhunt Tim Hunt added a comment -

            See also, MDL-7336 / http://docs.moodle.org/en/Development:Filter_enable/disable_by_context

            Note that http://docs.moodle.org/en/Development:Filters was not updated, which is OK, because I may be about to change things a bit.

            Show
            timhunt Tim Hunt added a comment - See also, MDL-7336 / http://docs.moodle.org/en/Development:Filter_enable/disable_by_context Note that http://docs.moodle.org/en/Development:Filters was not updated, which is OK, because I may be about to change things a bit.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  24/Nov/10