16 Mar
2009
16 Mar
'09
10:39 a.m.
Sergiu Dumitriu wrote:
jvelociter (SVN) wrote:
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.
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.
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).