Sergiu Dumitriu wrote:
> jvelociter (SVN) wrote:
>> Author: jvelociter
>> Date: 2009-03-14 23:32:39 +0100 (Sat, 14 Mar 2009)
>> New Revision: 17629
>>
>> Modified:
>>
>> platform/core/branches/xwiki-core-1.8/xwiki-core/src/main/java/com/xpn/xwiki/render/filter/XWikiHeadingFilter.java
>> Log:
>> XWIKI-3360 Including a document with first or second heading level breaks
>> the including document section edit links (Syntax 1.0)
>>
>> Internal count of section was incremented for headings of included documents
>> while they are not part of the document and thus not subject to section edit
>> in an included context. In doubt I left the comment related to the
>> increment, though I am not sure it is relevant.
>>
>>
>> Modified:
>> platform/core/branches/xwiki-core-1.8/xwiki-core/src/main/java/com/xpn/xwiki/render/filter/XWikiHeadingFilter.java
>> ===================================================================
>> ---
>> platform/core/branches/xwiki-core-1.8/xwiki-core/src/main/java/com/xpn/xwiki/render/filter/XWikiHeadingFilter.java
>> 2009-03-14 15:43:17 UTC (rev 17628)
>> +++
>> platform/core/branches/xwiki-core-1.8/xwiki-core/src/main/java/com/xpn/xwiki/render/filter/XWikiHeadingFilter.java
>> 2009-03-14 22:32:39 UTC (rev 17629)
>> @@ -158,15 +158,15 @@
>> }
>>
>> if (level.equals("1") || level.equals("1.1")) {
>> - // TODO: This is unstable, meaning that it works in the
>> current skin, but it might
>> - // fail if there are other headings processed before the
>> document content.
>> - sectionNumber++;
>> // This check is needed so that only the document content
>> generates sectionedit
>> // links.
>> // TODO: Find a better way to make this check, as this
>> prevents generating links for
>> // titles that are transformed by velocity (1.1 about
>> $doc.fullName) or by radeox
>> // (1.1 This is *important*).
>> if (doc != null && doc.getContent().indexOf(title.trim())
>> != -1) {
>> + // TODO: This is unstable, meaning that it works in the
>> current skin, but it might
>> + // fail if there are other headings processed before
>> the document content.
>> + sectionNumber++;
>> StringBuffer editparams = new StringBuffer();
>> if
>> (xcontext.getWiki().getEditorPreference(xcontext).equals("wysiwyg")) {
>>
>> editparams.append("xpage=wysiwyg&section=").append(sectionNumber);
>
> You just re-introduced http://jira.xwiki.org/jira/browse/XWIKI-1176
> Too bad there was no test for it (my fault).
>
> The problem is that the section edit buttons should be added just for
> real sections in the current document, but it is hard to do that. The
> current code does a simple check to see if the current header (wiki
> syntax) is found in the document. This raises two issues: since radeox
> is executed after velocity, if the document source contained a heading
> with velocity, this check raises a false negative. Now:
>
> - if we put the increment inside the if, the sections will be wrong
> because of this false negative
> - if we leave it outside, true negatives (like imported content or
> headers generated inside loops) will also shift section numbers
>
> So either way, the whole code is wrong.
Ok, I see. I think we should keep he increment inside the clause and either:
- Improve the check as the TODO suggests (but I don't really see how now)
- Decide that "1.1 About $doc.fullName" (or other title with
radeox/velocity) is not a section, and change Document#getSections to
reflect that (checking each title against a regular expressions for
example, and discard it if it contains velocity or radeox).
Having the increment outside the clause sounds wrong any case.
XWIKI-3360 can be experienced on the only edit link of Main.WebHome of a fresh
XE, so I prefer not to revert for now, and reopen XWIKI-1176 if needed.
Jerome.
>
> For reference, this was the change I made:
> http://fisheye2.atlassian.com/changelog/xwiki?cs=4485 (for some unknown
> reason svn fails to show the changes, although it says that 3 lines were
> added and one removed).
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
