Textarea Field, Multiline Block (from acbart)

[rachel-fenichel]

This is a version of #2584 from @acbart, with two changes:

• fixed call Blockly.Xml.utils, which we renamed while the other PR was open
• removed language files from the PR

@acbart I made a new PR because I didn't want to push to your develop branch.

The basics

• I branched from develop
• My pull request is against develop
• My code follows the style guide

The details

See #2584 for details and discussion.

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only "I consent." in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[acbart]

I consent.

[acbart]

Looks like there's a couple unit tests still failing with this. Do you have it under handled or does this need to be tackled again?

[rachel-fenichel]

If you want to take a look, that would be awesome. The failing unit test does seem to be directly related to the XML.

[acbart]

Just as an update, I've tracked it down to the XML unit test changing slightly. There's now an XML element without a closing tag. @NeilFraser added it a few weeks ago: https://github.com/google/blockly/blame/develop/tests/jsunit/xml_test.js#L38

Assuming this is desirable, I think I'll need to update the regex to handle this case. Which... doesn't look super fun, but I'll try to take a stab at it.

[acbart]

I believe the new regex should be:

var regexp = /(<[^/][^<]*>[^<]*[^/])\n([^<]*<\/)/;

In core/xml.js:

blockly/core/xml.js

Line 299 in 3f66116

var regexp = /(<[^/][^<]*>[^<]*)\n([^<]*<\/)/;

But I am not entirely certain. It does pass the tests and my experimentation with a regex tool.

[rachel-fenichel]

We haven't forgotten this--Neil will take a look at the regex when he gets a chance.

[NeilFraser]

I believe the new regex should be:

var regexp = /(<[^/][^<]*>[^<]*[^/])\n([^<]*<\/)/;

In core/xml.js:

blockly/core/xml.js

Line 299 in 3f66116

var regexp = /(<[^/][^<]*>[^<]*)\n([^<]*<\/)/;

But I am not entirely certain. It does pass the tests and my experimentation with a regex tool.

Oh great. This is the exact issue raised in the most-cited Stack Overflow question of all time:
https://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags/1732454
Every engineer needs to read the first answer in its entirety. :)

But accepting our fate that we can't solve the general case, we can still solve the Blockly-specific one. The following example shows that your proposed regexp isn't correct:

'<foo>\n</foo>'.replace(/(<[^/][^<]*>[^<]*[^/])\n([^<]*<\/)/, '$1&#10;$2');

The most straight-forward regexp I can come up with is:

var regexp = /(<[^/](?:[^>]*[^/])?>[^<]*)\n([^<]*<\/)/;

This would be easier if browsers supported lookbehind assertions, but so far only Chrome supports them.

Definitely also add another example to the comment:

  // E.g. <foo>\n<bar/>\n</foo> is unchanged.

[samelhusseini]

An update on this. I have gone through and updated the field so that:

• It uses ES5
• The closure typings are all correct
• Better alignment of the field
• Use CSS for static style properties
• Works in Edge and IE (Use text with a group instead of tspan).

This is ready for a final review. There's a CLA Issue where it's unable to verify concent. @rachel-fenichel do you have any idea how to resolve?

[rachel-fenichel]

CLA is fine--he consented above: #2663 (comment)

[rachel-fenichel]

I didn't know you could do this.

[samelhusseini]

Also, the round trip jsunit tests are failing which I'm looking into.

[samelhusseini]

Fixed jsunit tests by using Neil's regex instead.

[rachel-fenichel]

Field structure lgtm.