On Oct 28, 2008, at 2:22 PM, Thomas Mortagne wrote:
> On Tue, Oct 28, 2008 at 3:12 AM, Vincent Massol <[EMAIL PROTECTED]>
> wrote:
>>
>> On Oct 28, 2008, at 11:07 AM, Sergiu Dumitriu wrote:
>>
>>> vmassol (SVN) wrote:
>>>> Author: vmassol
>>>> Date: 2008-10-28 09:19:23 +0100 (Tue, 28 Oct 2008)
>>>> New Revision: 13864
>>>>
>>>> Added:
>>>> platform/core/trunk/xwiki-rendering/src/test/java/org/xwiki/
>>>> rendering/block/XMLBlockTest.java
>>>> Modified:
>>>> platform/core/trunk/xwiki-rendering/src/main/java/org/xwiki/
>>>> rendering/block/XMLBlock.java
>>>> Log:
>>>> XWIKI-2785: The order in which XML attributes are printed when
>>>> converted to XWiki syntax is not fixed
>>>>
>>>
>>>> + public void testAttributeOrder()
>>>> + {
>>>> + Map<String, String> attributes = new HashMap<String,
>>>> String>();
>>>> + attributes.put("aaa", "value1");
>>>> + attributes.put("bbb", "value2");
>>>> + XMLBlock block = new XMLBlock("element", attributes);
>>>> +
>>>> + Iterator<String> it = attributes.keySet().iterator();
>>>> + Iterator<String> newIt =
>>>> block.getAttributes().keySet().iterator();
>>>> + while (it.hasNext()) {
>>>> + assertEquals(newIt.next(), it.next());
>>>> + }
>>>> + }
>>>
>>> I'm not sure this test is enough,
>>
>> it's definitely not enough.
>>
>>> since the order for two attributes
>>> could happen to be the same. Why not add 10 attributes (aN = vN,
>>> where N
>>> goes from 0 to 9)?
>>
>> I've just tried this:
>>
>> public void testAttributeOrder()
>> {
>> Map<String, String> attributes = new HashMap<String,
>> String>();
>> for (int i = 0; i < 10; i++) {
>> attributes.put("key" + i, "value" + i);
>> }
>> XMLBlock block = new XMLBlock("element", attributes);
>>
>> Iterator<String> it = attributes.keySet().iterator();
>> Iterator<String> newIt =
>> block.getAttributes().keySet().iterator();
>> while (it.hasNext()) {
>> assertEquals(newIt.next(), it.next());
>> }
>> }
>>
>> And the tests till passes even when I revert the implementation for
>> XMLBlock (i.e. no usage of LinkedHashMap).
>>
>> Any more idea? Could it be that the test is wrong or the
>> implementation not correct? (I don't se anything wrong).
>
> It's the implementation :
> change
>> this.attributes = attributes;
> by
>> this.attributes = new LinkedHashMap<String, String>(attributes);
>
> does not change anything, the most important is what is the initial
> Map implementation of provided "attributes":
> - if it's HashMap then there is nothing you can do here to have the
> same order user set in the wiki syntax
> - if it's already LinkedHashMap then you have nothing to do
I'm not sure. I thought about this too. Here's my understanding: we
should return the map keyset in the same order as provided by the
user, i.e. if the user uses a HashMap then we return it in the same
order as the HashMap. If the user uses a LinkedHashMap then we return
it in that order too. I also think we should create a new map and not
use the one provided by the user. I was also hesitating about adding a
addAttribute() method to the class.
Note that I was not talking about end users but about users of XMLBlock.
Last, I've also fixed XHTMLMacro locally so that it uses a
LinkedHashMap when creating XMLBlock elements.
Thanks
-Vincent
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
