18 Feb
2015
18 Feb
'15
4:20 p.m.
On 27 Nov 2014 at 18:46:50, Eduard Moraru
(enygma2002@gmail.com(mailto:enygma2002@gmail.com)) wrote:
Greetings from the far future! No, the new model is
sadly not here yet, but
we have a shiny new UI :)
Bumping this old thread since I`ve stumbled upon this issue recently and it
took me a while to understand what the actual problem is (thanks Thomas!)
and I feel it's something we really need to get fixed as soon as possible.
Thomas was suggesting we add this on the list for the 7.x Roadmap and I`m
obviously +1 for that.
There is the matter of backwards compatibility, that we need to still
address, as either to break it or keep it. My take would be to break it,
since I consider it to be bad code to depend on this broken behavior in the
first place and that huge security hole we keep dragging along needs to be
plugged abruptly.
WDYT?
Since this is available from our scripting api I don’t think we should break it, or we can
be sure we’re going to break some extensions.
An alternative is to move the getRenderedContent() to legacy and implement a new method
and use that new method in our code.
The only issue with this is the naming of the new method and the closeness with the other
getRenderedContent(…) signatures…
We could also introduce new methods to replace all the getRenderedContent(…) methods and
move them all to legacy.
Thanks
-Vincent
Thanks,
Eduard
On Fri, Sep 4, 2009 at 11:56 AM, Thomas Mortagne
wrote:
> On Fri, Sep 4, 2009 at 10:39, Vincent Massol wrote:
> >
> > On Sep 4, 2009, at 10:30 AM, Thomas Mortagne wrote:
> >
> >> On Fri, Sep 4, 2009 at 10:07, Vincent Massol
> >> wrote:
> >>> Hi,
> >>>
> >>> Im trying to fix http://jira.xwiki.org/jira/browse/XWIKI-4274
> >>>
> >>> Basically if you do
> >>> $xwiki.getDocument("someDoc").getRenderedContent()
> >>> it'll get executed in the context of the current doc which I
believe
> >>> is wrong especially since other signatures of getRenderedContent()
> >>> execute in the target document's context.
> >>>
> >>> I have fixed this locally but found that admin.vm for example is
> >>> assuming that getRenderedContent() will get executed in the context
> >>> of
> >>> the calling doc (i.e. XWiki.Import when doing an import for example).
> >>> FYI the chain flow is admin.vm -- getRenderedContent() -->
> >>> XWiki.AdminSheet --> XWiki.AdminImportSheet --> importinline.vm,
> >>> which
> >>> requires the current doc to be XWiki.Import (to get/put attachments
> >>> from/to it).
> >>> I can fix this easily using a new getRenderedContent signature
I've
> >>> introduced.
> >>>
> >>> However I'm wondering if we have other places that incorrectly use
> >>> getRenderedContent() and assume it won't be rendered in the
context
> >>> of
> >>> the target document.
> >>>
> >>> Is this change too dangerous to make? If not know, we'll need to
it
> >>> quickly (2.1M1?) since it's an important bug IMO.
> >>
> >> Yes it's a dangerous fix I think, getRenderedContent() is really not
> >> an hidden method so changing its behavior that much even if it's a fix
> >> is dangerous anyway.
> >
> > But would you be ok to make this change in 2.1M1?
> >
> > Or would you prefer we live with it and instead document that it
> > doesn't execute in the document's contex (and instead add a new method
> > that does)? It's much simpler for sure but keeps adding to the mess.
> > We could decide to keep adding to the mess and fix it when we rewrite
> > that part in some far future.
>
> Actually i was mainly thinking that we will rewrite all that in the
> new model anyway so for me one way or another it's temporary code so i
> think i would be more in favor of not changing the behavior of the
> current getRenderedContent() and instead add new one for backward
> compatibility.
>
> >
> > Thanks
> > -Vincent