node-set($x), when $x is empty, should not halt XSLT processing

node-set($x), when $x is empty, should not halt XSLT processing

node-set is polymorphic; if called with a string, it returns a
node-set containing a text node with that content. The libxslt roll in
r425586 broke this behavior because Blink's fork of the node-set
extension returned a text node without an owner document. Fix that.

BUG=689977
Review-Url: https://codereview.chromium.org/2750943004
Cr-Commit-Position: refs/heads/master@{#457692}
Committed: https://chromium.googlesource.com/chromium/src/+/3f6e482b7f13c1fc5bd68227321ef6da0ddf9937

[dominicc (has gone to gerrit), 2017-03-15 07:09:33]

dominicc@chromium.org changed reviewers:
+ yosin@chromium.org

[dominicc (has gone to gerrit), 2017-03-15 07:09:34]

PTAL

[dominicc (has gone to gerrit), 2017-03-15 07:36:27]

The CQ bit was checked by dominicc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 07:36:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yosin_UTC9, 2017-03-15 07:47:24]

C++ changes are OK because it is the copy of
third_party/libxslt/libexslt/common.c
I think we can improve test to check result of exslt::node-set('') rather than
checking whether this test crash or not.

https://codereview.chromium.org/2750943004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/xsl/xslt-node-set-empty.html (right):

https://codereview.chromium.org/2750943004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/xsl/xslt-node-set-empty.html:10:
exclude-result-prefixes="exsl">
We don't need to have exclude-result-prefixes since this test doesn't intent to
test this feature.

https://codereview.chromium.org/2750943004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/xsl/xslt-node-set-empty.html:13:
<xsl:variable name="look" select="exsl:node-set($empty)"/>
Can we write this just |exsl:node-set('')|?

https://codereview.chromium.org/2750943004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/xsl/xslt-node-set-empty.html:16:
<xsl:text>PASS</xsl:text>
Rather than checking whether crash or not, checking value return of of
exsl:node-set() is more useful.

e.g

string-length(node-set('')) = <xsl:value-of select="string-length(node-set('')">

https://codereview.chromium.org/2750943004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/xsl/xslt-node-set-empty.html:24: let xslproc
= new XSLTProcessor();
nit: s/let/const/

https://codereview.chromium.org/2750943004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/xsl/xslt-node-set-empty.html:26: let result
= xslproc.transformToFragment(xml, document);
nit: s/let/const/

[commit-bot: I haz the power, 2017-03-15 11:03:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 11:03:14]

Dry run: This issue passed the CQ dry run.

[dominicc (has gone to gerrit), 2017-03-16 09:21:47]

The CQ bit was checked by dominicc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 09:22:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominicc (has gone to gerrit), 2017-03-16 09:23:09]

The CQ bit was checked by dominicc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-16 09:23:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominicc (has gone to gerrit), 2017-03-16 09:23:42]

Thanks for the review, PTAL.

https://codereview.chromium.org/2750943004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/xsl/xslt-node-set-empty.html (right):

https://codereview.chromium.org/2750943004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/xsl/xslt-node-set-empty.html:13:
<xsl:variable name="look" select="exsl:node-set($empty)"/>
On 2017/03/15 at 07:47:24, yosin_UTC9 wrote:
> Can we write this just |exsl:node-set('')|?

Yes, done.

https://codereview.chromium.org/2750943004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/xsl/xslt-node-set-empty.html:16:
<xsl:text>PASS</xsl:text>
On 2017/03/15 at 07:47:24, yosin_UTC9 wrote:
> Rather than checking whether crash or not, checking value return of of
exsl:node-set() is more useful.
> 
> e.g
> 
> string-length(node-set('')) = <xsl:value-of
select="string-length(node-set('')">

This unfortunately doesn't work. The thing which halts processing prematurely is
the function that's evaluating the expressions to populate variables; in a
value-of expression, node-set('') works.

[yosin_UTC9, 2017-03-16 09:37:22]

lgtm

Thanks for explanation.

[commit-bot: I haz the power, 2017-03-16 11:01:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-16 11:01:54]

Dry run: This issue passed the CQ dry run.

[dominicc (has gone to gerrit), 2017-03-17 05:10:37]

The CQ bit was checked by dominicc@chromium.org

[commit-bot: I haz the power, 2017-03-17 05:10:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-17 05:16:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489727437711590,
"parent_rev": "0bc5ffca0d9756f49fba26e03fcdf8c3d58877c1", "commit_rev":
"3f6e482b7f13c1fc5bd68227321ef6da0ddf9937"}

[commit-bot: I haz the power, 2017-03-17 05:17:36]

Description was changed from

==========
node-set($x), when $x is empty, should not halt XSLT processing

node-set is polymorphic; if called with a string, it returns a
node-set containing a text node with that content. The libxslt roll in
r425586 broke this behavior because Blink's fork of the node-set
extension returned a text node without an owner document. Fix that.

BUG=689977
==========

to

==========
node-set($x), when $x is empty, should not halt XSLT processing

node-set is polymorphic; if called with a string, it returns a
node-set containing a text node with that content. The libxslt roll in
r425586 broke this behavior because Blink's fork of the node-set
extension returned a text node without an owner document. Fix that.

BUG=689977

Review-Url: https://codereview.chromium.org/2750943004
Cr-Commit-Position: refs/heads/master@{#457692}
Committed:
https://chromium.googlesource.com/chromium/src/+/3f6e482b7f13c1fc5bd68227321e...
==========

[commit-bot: I haz the power, 2017-03-17 05:17:37]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/3f6e482b7f13c1fc5bd68227321e...