Jackrabbit Content Repository
  1. Jackrabbit Content Repository
  2. JCR-3077

WeightedHighlighter does not encode XML markup characters

    Details

      Description

      See JCR-2611; the same problem applies to WeightedHighlighter.

      1. JCR-3077.patch
        8 kB
        Julian Reschke
      2. JCR-3077.patch
        9 kB
        Julian Reschke
      3. JCR-3077.patch
        9 kB
        Julian Reschke

        Activity

        Hide
        Jukka Zitting added a comment -

        Merged to the 1.6 branch in revision 1174650.

        Show
        Jukka Zitting added a comment - Merged to the 1.6 branch in revision 1174650.
        Hide
        Jukka Zitting added a comment -

        Merged to the 2.1 branch in revision 1174630 and to 2.0 in revision 1174642.

        Show
        Jukka Zitting added a comment - Merged to the 2.1 branch in revision 1174630 and to 2.0 in revision 1174642.
        Hide
        Julian Reschke added a comment -

        2.2: r1173250

        Show
        Julian Reschke added a comment - 2.2: r1173250
        Hide
        Julian Reschke added a comment -

        Fixed with r1173196

        Show
        Julian Reschke added a comment - Fixed with r1173196
        Hide
        Alex Parvulescu added a comment -

        cool!

        +1 for the patch then.

        Show
        Alex Parvulescu added a comment - cool! +1 for the patch then.
        Hide
        Julian Reschke added a comment -

        > a tiny feedback on your patch

        Merci.

        > I think it would be better if the test was in the "ExcerptTest" class.

        Yep; I just did that.

        > I like the "escape" refactoring.
        >Why is it not static? Text.encodeIllegalXMLCharacters is static, what is the gain of adding a non-static method to call a static one?

        It needs to be non-static to that classes that extend from the DefaultHighLighter can override it.

        > What is the benefit of making methods non static?

        So that they can call the other methods I made non-static for the reason above

        Show
        Julian Reschke added a comment - > a tiny feedback on your patch Merci. > I think it would be better if the test was in the "ExcerptTest" class. Yep; I just did that. > I like the "escape" refactoring. >Why is it not static? Text.encodeIllegalXMLCharacters is static, what is the gain of adding a non-static method to call a static one? It needs to be non-static to that classes that extend from the DefaultHighLighter can override it. > What is the benefit of making methods non static? So that they can call the other methods I made non-static for the reason above
        Hide
        Julian Reschke added a comment -

        updated patch, uses ExcerptTest instead

        Show
        Julian Reschke added a comment - updated patch, uses ExcerptTest instead
        Hide
        Alex Parvulescu added a comment -

        a tiny feedback on your patch

        > - adds a test case
        I think it would be better if the test was in the "ExcerptTest" class.

        > - puts the convenience method "escape" into the base class (and uses it throughout)
        I like the "escape" refactoring.
        Why is it not static? Text.encodeIllegalXMLCharacters is static, what is the gain of adding a non-static method to call a static one?

        > - removes "static" from two methods on WeightedHighlighter
        What is the benefit of making methods non static?

        Show
        Alex Parvulescu added a comment - a tiny feedback on your patch > - adds a test case I think it would be better if the test was in the "ExcerptTest" class. > - puts the convenience method "escape" into the base class (and uses it throughout) I like the "escape" refactoring. Why is it not static? Text.encodeIllegalXMLCharacters is static, what is the gain of adding a non-static method to call a static one? > - removes "static" from two methods on WeightedHighlighter What is the benefit of making methods non static?
        Hide
        Julian Reschke added a comment -

        (whitespace fixed)

        Show
        Julian Reschke added a comment - (whitespace fixed)
        Hide
        Julian Reschke added a comment -

        see prev. comment

        Show
        Julian Reschke added a comment - see prev. comment
        Hide
        Julian Reschke added a comment -

        This patch

        • adds a test case
        • puts the convenience method "escape" into the base class (and uses it throughout)
        • fixes a Javadoc typo
        • fixes WeightedHighlighter to invoke the escaping as well
        • removes "static" from two methods on WeightedHighlighter
        Show
        Julian Reschke added a comment - This patch adds a test case puts the convenience method "escape" into the base class (and uses it throughout) fixes a Javadoc typo fixes WeightedHighlighter to invoke the escaping as well removes "static" from two methods on WeightedHighlighter

          People

          • Assignee:
            Julian Reschke
            Reporter:
            Julian Reschke
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development