Oh, wait a minute. I did believe we've got only type="text" and type="html" and not type="string". That's a bit wrong. I need to re-organize my thoughts and respond later...

On 5/25/2010 9:28 PM, P T Withington wrote:
My proposed change is only for<attribute type="string" />, which is currently 
broken and the source of LPP-9027, so perhaps we should review that separately.

I agree that the handling of node text is a mess.  A lot of the mess is due to 
ancient cruft in the Text compiler that was only necessary for swf5 and should 
be ripped out.  Doing so would surely simplify the problem.

I'm a little confused though:  Am I missing something?  I don't see where we do any handling of attributes that might 
be declared to be of type "text" or "html".  Seems like it would be a great improvement if we did 
though.  Maybe we'd have less confusion if we called these types "cdata" and "content"?

I can see also that the current mechanism for handling the content of a node in 
NodeModel is broken, as you point out, because it is using ScriptCompiler.quote 
on the values in addText.  I agree this is incorrect.

I propose 1) you review my change for it's limited fix, 2) we file an 
improvement to rip out the swf5 text machinations, and 3) we file a bug 
regarding NodeModel/addText doing incorrect escaping of node content.

On 2010-05-25, at 11:02, André Bargull wrote:

I don't think this is right. If you set the value of an attribute with type="text", 
you need to use a 'xml-string' [1,2]. Also for Unicode characters you must not use a 
Javascript escape sequence \uXXXX, but the corresponding the character reference&#XXXX; 
[2].
(People who don't like xml and xml character references should use 
type="expression" instead, that way they can write a 'javascript-string' [1] 
with Javascript escape sequences etc. :-) )
I know this is a confusing topic [4], it's not always clear when to apply which escaping. But as said, also after reading the 
source code in NodeModel.java, it's clear that type="text" and type="html" should not treat 
the<attributes>'s value as a Javascript string. It might not be clear for type="text", but at least for 
type="html" it's obvious the value is a (x)html string, so no Javascript escape characters [5]!

It's seems you want to have type="string" which treats the value as a 
Javascript string.

And the change could possibly break applications because \ is no longer escaped 
to \\.



[1] not an official term, only used here to make clear we've got 'xml-strings' 
and 'javascript-strings'
[2] http://www.w3.org/TR/REC-xml/#NT-AttValue
[3] http://www.w3.org/TR/REC-xml/#NT-CharRef
[4] http://jira.openlaszlo.org/jira/browse/LPP-1948
[5] the only difference between type="text" and type="html" is how to interpret 
text-content (= CharData http://www.w3.org/TR/REC-xml/#NT-CharData)


On 5/25/2010 4:04 PM, P T Withington wrote:
Change 20100525-ptw-1 by [email protected] on 2010-05-25 09:56:35 EDT
     in /Users/ptw/OpenLaszlo/trunk-2
     for http://svn.openlaszlo.org/openlaszlo/trunk

Summary: Correct quoting of string-typed attributes

Bugs Fixed: LPP-9027 pattern attribute on text doesn't work as expected in trunk

Technical Reviewer: [email protected] (pending)
QA Reviewer: [email protected] (pending)

Details:
     lztest-node:  Added tests to verify that setting a string
     attribute using a literal or expression is the same as setting an
     expression attribute to a literal string using the failing pattern
     from the bug (which verifies the use of \u escapes to specify
     unicode characters works).

     NodeModel:  Before you quote a string literal, be sure to parse it
     as an ECMAScript string literal (so that the ECMA \-escapes are
     properly interpolated).

Tests:
     ant lztest with new node attribute tests

Files:
M       test/lztest/lztest-node.lzx
M       WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java

Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20100525-ptw-1.tar



Reply via email to