Re: [xwiki-devs] [xwiki-notifications] r17629 - platform/core/branches/xwiki-core-1.8/xwiki-core/src/main/java/com/xpn/xwiki/render/filter
15 Mar
2009
15 Mar
'09
midnight
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.
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).
--
Sergiu Dumitriu
http://purl.org/net/sergiu/
16 Mar
16 Mar
10:39 a.m.
New subject: [xwiki-devs] [xwiki-notifications] r17629 - platform/core/branches/xwiki-core-1.8/xwiki-core/src/main/java/com/xpn/xwiki/render/filter
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).
17 Mar
17 Mar
3:29 p.m.
New subject: [xwiki-devs] [xwiki-notifications] r17629 - platform/core/branches/xwiki-core-1.8/xwiki-core/src/main/java/com/xpn/xwiki/render/filter
Jerome Velociter wrote:
Sergiu Dumitriu wrote:
OK for 2. There is little chance of finding a better fix, since regular
expressions are a very bad foundation.
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). 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. 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).
--
Sergiu Dumitriu
http://purl.org/net/sergiu/
5715
days inactive
5718
days old
2 comments
2 participants
participants (2)
-
Jerome Velociter -
Sergiu Dumitriu