I'm going to check this in ahead of final review because it (claims to) fix the
top priority webtop bug.
There have been 2 rounds of review already, so I feel it is relatively correct.
If there are further review issues I will re-open and address with a
subsequent change.
On 2010-06-17, at 10:25, P T Withington wrote:
> [UPDATED^3
>
> On 2010-06-16, at 12:10, André Bargull wrote:
>
>> ViewSchema#getTypeForName():
>>> if (name.equals("text") ||
>>> name.equals("html"))
>>> - name = "string";
>>> + name = "text";
>>
>> A bit simpler:
>> if ("html".equals(name)) {
>> // [insert explanatory comment here]
>> name = "text";
>> }
>
> Fixed
>
>>> + html: StringPresentationType,
>>> + text: TextPresentationType,
>>
>> 1) If text and html are treated as synonyms for now, they should have the
>> same presentation type.
>> 2) test/smoke/presentation-types.lzl needs to be updated to test the new
>> types, too.
>
> Fixed
>
>> In TextPresentationType.present():
>>> + i = e + 1;
>>
>> i = e? e is the position of the ';', e+1 is the next character which needs
>> to be processed, but as the for-loop increments i another time, the next
>> character which is processed is at position e+2, so the char value[e+1] is
>> left out.
>
> Thanks! Fixed, although TextPresentationType will be unused for now.
>
>> NodeModel#compileAttribute():
>>> + // Immediate string, token, and id types are
>>> + // auto-quoted, but must _first_ be parsed as ES
>>> + // strings!
>>
>> I'm not sure whether this is valid for IDs and tokens. Have you tested this?
>> For example this test doesn't compile at all, that means the compiler
>> doesn't not parse the values as ES strings:
>> <canvas>
>> <!-- name's type is token, id's type is ID -->
>> <view name="\x60" id="\x61"/>
>> </canvas>
>
> You are correct, these are XML tokens and ids. Fixed.
>
>>> + } else if (type == ViewSchema.XML_CDATA_TYPE
>>> + || type == ViewSchema.XML_CONTENT_TYPE) {
>>> + // Immediate text and html types are auto-quoted; They
>>> + // will be treated differently in the runtime
>>> + // PresentationType system
>>
>> This code is a bit misleading, because right know html-type is
>> auto-converted to text-type (ViewSchema#getTypeForName() !). An additional
>> comment should be added for clarification.
>
> Fixed with a comment.
>
>> Some components need to be updated to change the type from string to text
>> for backward compatibility. I'd suggest to do this in a subsequent change
>> set. For example in lz/windowpanel.lzx, the "title" attribute is set to
>> string, but it's only used in a constraint for a <text> element. I think in
>> this case the type for "title" should be changed to text.
>
> http://jira.openlaszlo.org/jira/browse/LPP-9126
>
>> And it'd be nice if docs/src/developers/methods-events-attributes.dbk was
>> updated to reflect the new understanding of string/text/html.
>
> Fixed.
>
> ]
>
> Change 20100525-ptw-1 by [email protected] on 2010-05-25 09:56:35 EDT
> in /Users/ptw/OpenLaszlo/trunk
> 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)
>
> Release Notes:
>
> The LZX `string` attribute type is no longer a synonym for `text`
> and `html`. The `string` type implies an ECMAScript String.
> Literal values assigned to attributes of that type will be
> interpreted as ECMAScript `String` literals, meaning that the
> ECMAScript single-character (\t, \n, etc.) and short (\xNN) and
> long (\uNNNN) escapes will be interpreted as defined in the
> ECMAScript standard. The `text` and `html` types imply XML
> content. Literal values assigned to attributes of that type will
> (continue to) be interpreted as verbatim text. ECMAScript escapes
> will not be interpreted.
>
> The `<text>/text` attribute is now declared to be of type `text`,
> to indicate that it represents HTML content, not an ECMAScript
> string. If you redeclare the `text` attribute in a subclass of
> `<text>` you should omit any type declaration as it is redundant.
> (If you had declared the type as `string`, the compiler will issue
> a warning.)
>
> At present `text` and `html` are still synonyms, but this is a
> bug. The intent is that `text` represents XML CDATA (with the
> implication that markup is _not_ interpreted), whereas `html`
> represents XML content (with the implication that markup _is_
> interpreted). See LPP-1948.
>
> Overview:
>
> Split the `string` LZX type from the `text` and `html` types
> (leaving the latter two as synonyms for now). Declare the type of
> `<text>/text` to be `text` (which is the popular concensus in
> components, although perhaps incorrect because <text> behaves as
> if it were `html`; that is, it interprets markup).
>
> With this split, we are able to have the LZX compiler interpret
> literal `string` values as ECMAScript `String`s and solve the
> original bug. In a future change we will address the
> `<text>/text` `text`/`html` issue.
>
> 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).
>
> presentation-types: Added tests for `html` and `text` types and
> enhanced test for `string` type to ensure String escapes work as
> expected.
>
> LzNode: Added mapping for text and html presentation types..
>
> PresentationTypes: Added text presentation type which
> escapes/unescapes the 5 XML entities. This presentation type
> should eventually replace the many copies of xml escaping code.
>
> LzText: Change the @lzxtype of the `text` attribute from `string`
> to `text`.
>
> LzDataset: Correct spelling of @lzxtype noticed in passing
>
> SchemaBuilder: Handle `text` and `html` types
>
> Schema, ViewSchema, NodeModel: Added real types for 'text' and
> 'html', distinct from 'string', which affects how immediate values
> of those types are interpreted by the tag compiler: 'string'
> implies an ECMAScript string (i.e., interpreted as an ECMAScript
> String literal), 'text' implies XML CDATA and 'html' implies XML
> content. In this change, the latter two remain synonyms (as they
> currently are), and are _not_ interpreted as ECMAScript strings,
> but as XML content. In a later change the implied distinction
> between `text` and `html` will be addressed
>
> methods-events-attributes.dbk: Update type table to reflect
> current understanding.
>
> debugger: Remove unused `escapeText` noticed in passing.
>
> basecombobox: Remove redundant (conflicting) attribute
> declaration.
>
> photo, classes: Remove redundant (conflicting) attrbute type
> specification.
>
> Tests:
> ant lztest with new node attribute tests, smokecheck, various
> combinations of demos and runtimes.
>
> Files:
> M test/lztest/lztest-node.lzx
> M test/smoke/presentation-types.lzl
> M WEB-INF/lps/lfc/core/LzNode.lzs
> M WEB-INF/lps/lfc/core/PresentationTypes.lzs
> M WEB-INF/lps/lfc/views/LzText.lzs
> M WEB-INF/lps/lfc/data/LzDataset.lzs
> M WEB-INF/lps/server/src/org/openlaszlo/js2doc/SchemaBuilder.java
> M WEB-INF/lps/server/src/org/openlaszlo/xml/internal/Schema.java
> M WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewSchema.java
> M WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java
> M docs/src/developers/methods-events-attributes.dbk
> M lps/components/debugger/debugger.lzx
> M lps/components/base/basecombobox.lzx
> M demos/lzpix/classes/photo.lzx
> M demos/lzpix/classes/classes.lzx
> M demos/lzpix/classes/clipboard.lzx
>
> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20100525-ptw-1.tar
