Wrappers don't wrap everything what should be wrapped (MAGNOLIA-4810)

[MAGNOLIA-4866] Make sure every node and property returned by HTML or I18N wrappers are wrapped Created: 24/Feb/13  Updated: 13/Feb/17  Resolved: 27/Feb/13

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: 4.5
Fix Version/s: 4.5.8

Type: Sub-task Priority: Critical
Reporter: Jan Haderka Assignee: Jan Haderka
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to MAGNOLIA-6957 I18nNodeWrapper no longer wraps child... Closed
dependency
is depended upon by MGNLSTK-1095 Escape values for rendering, don't es... Closed
is depended upon by MGNLSTK-1105 Escape values for rendering, don't es... Closed
Template:
Date of First Response:

 Description   

Those two wrappers are most often used and as such most critical.
Also while looking at the problem I've found that whole wrapping framework is duplicated in info.magnolia.jcr.decoration and info.magnolia.jcr.wrapper (both created in about same timeframe during 4.5 development). As a result ChildWrappingNodeWrapper and related classes are deprecated and the ones from ...decoration package will be promoted from now on.



 Comments   
Comment by Tobias Mattsson [ 25/Feb/13 ]

I would prefer if the ContentDecorator classes for escaping and i18n were top level public classes to promote using them as:

new I18nContentDecorator().wrapNode(node) 

This will add new constructors to HTMLEscapingNodeWrapper, HTMLEscapingPropertyWrapper and I18nNodeWrapper. The existing ones are kept but deprecated.

I'm note so convinced we should deprecate ChildWrappingNodeWrapper, its still a viable option when the full decoration offered by ContentDecorator isn't necessary.

Comment by Jan Haderka [ 27/Feb/13 ]

Fair enough, externalized the decorators. Also added some generics to it to make use easier. And exposed decorator in property wrapper (was already exposed in the node wrapper).
As for deprecation of other wrappers - there's actually just very few cases when you don't need to wrap everything. I would prefer ppl explicitly declaring that in their wrappers rather then choosing one of many different wrappers w/o really knowing how much they need (or not) to escape. Plus having 2 frameworks for same thing introduces maintenance overhead and is confusing for users.

Comment by Jan Haderka [ 03/Mar/13 ]

commits https://git.magnolia-cms.com/gitweb/?p=magnolia_main.git;a=commit;h=450295ad070e7560e8f0684256b1c7c48eb7391c and https://git.magnolia-cms.com/gitweb/?p=magnolia_main.git;a=commit;h=981acd7d9cda24069baaa6589ca50250d5bbd057 were accidentally made under MAGNOLIA-4810.

Generated at Mon Feb 12 03:59:46 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.