search

SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.96k stars 1.24k forks source link

Curly braces cause SyntaxError in sap.m.MultiInput under certain conditions #3175

Closed antonborisoff closed 3 years ago

antonborisoff commented 3 years ago

OpenUI5 version: 1.87.0

Browser/version (+device/version): Chrome 88.0.4324.150 (Official Build) (64-bit)

Any other tested browsers/devices(OK/FAIL): No

URL (minimal example if possible): jsfiddle: https://jsfiddle.net/dc53ao21/

User/password (if required and possible - do not post any confidential information here):

Steps to reproduce the problem:

  1. Go to the link
  2. Open browser console
  3. Copy and paste the following string: curly braces {{ 1
  4. Once the suggestion popover is open, press outside the control to trigger focusleave

What is the expected result? No SyntaxError, token is present in MultiInput

What happens instead? No token in MultiInput, SyntaxError in console:

BindingParser-dbg.js:363 Uncaught SyntaxError: no closing braces found in 'curly braces {{ 1' after pos:14 at q (sap-ui-core.js:295) at Function.a.complexParser [as bindingParser] (sap-ui-core.js:298) at f.o.extractBindingInfo (sap-ui-core.js:399) at f.o.applySettings (sap-ui-core.js:362) at sap-ui-core.js:357 at f.constructor (sap-ui-core.js:357) at f.constructor (sap-ui-core.js:996) at f.constructor (sap-ui-core.js:821) at new f (sap-ui-core.js:581) at f.n._configureTokenOptions (library-preload.js:3526) image

Any other information? (attach screenshot if possible) It seems the issue is similar to other issues with curly braces, for example:

2445

2233

Thus the proposed solution is to use ManagedObject.ManagedObject.escapeSettingsValue in _configureTokenOptions as it was done in other cases.

Please down port to 1.71.x and also for the sapui5 version

boghyon commented 3 years ago

Please down port to 1.71.x

Works well in SAPUI5 1.71.34, OpenUI5 1.71.29, and with the current nightly build.
I guess the change in https://github.com/SAP/openui5/commit/b9e64f87c9098569f69b47f5e268a8a7400e6a20 will be included in the next patch version of the current stable release.

codeworrior commented 3 years ago

Correct. db9717fda021e0b2dd41222151d7fded1c1961e9 is the corresponding commit for the rel-1.84 branch. For 1.87, the fix was too late, as far as I can see.

antonborisoff commented 3 years ago

Hi @boghyon .

I've just tested OpenUI5 1.71.29 by specifying the release version in the URL and SyntaxError error still occurs. Please, note that the fix you mentioned fixes only one method (_onSuggestionItemSelected) and not the other one (_validateCurrentText in the old version or _configureTokenOptions in the latest one) where the issue occurs: image

antonborisoff commented 3 years ago

Hello @boghyon , @codeworrior . Are there any updates regarding this issue? There weren't any updates (any labels or replies) for a couple of days.

terezamch commented 3 years ago

Hello @antonborisoff ,

the issue is fixed for version 1.71 in patch set 34. Therefore in the previous patch sets the issue is still present.

Regards, Tereza

terezamch commented 3 years ago

Hello @antonborisoff ,

I have asked the expert team on the topic whether the same approach should be applied in the other places where new Token is created. The internal incident is with id 2170006168.

Thanks and best regards, Tereza

d3xter666 commented 3 years ago

Hi @antonborisoff ,

The issue has been already fixed in the following duplicate issue: https://github.com/SAP/openui5/issues/3156

Cheers

boghyon commented 3 years ago

@d3xter666 It's unfortunately still reproducible when following the steps mentioned in the issue description, even in the nightly build.

antonborisoff commented 3 years ago

Hi @d3xter666 . Unfortunately, the issue is still reproducible in jsfiddle provided, although the latest version (1.87) is used. I've also checked the fix you linked and can't see any possible way it could have fixed the issue since the issue itself occurs in MultiInput method _validateCurrentText (as you can see from the screenshot), which does not involve any Input base methods. It seems that the issue should be reopened and addressed.

d3xter666 commented 3 years ago

Hi @antonborisoff,

Could you provide more details about the version you're testing now. Here's a fiddle with the UI5 nightly where the issue does not occur: https://jsfiddle.net/nehz52r9/

Can you check there and provide steps to reproduce.

antonborisoff commented 3 years ago

Hi @d3xter666 . Just follow the steps in your fiddle, but for step 4 tap on CSS tab, for example. After that you will get the issue mentioned here: https://github.com/SAP/openui5/issues/3153 It seems that scenario B from there was not fixed and _configureTokenOptions method was not adapted. If this method is adapted, you should start to get the error described in the current issue (_validateCurrentText method). I hope it helps.

antonborisoff commented 3 years ago

Hello @d3xter666 , Were you able to reproduce the issue?

d3xter666 commented 3 years ago

Hi @antonborisoff,

Yes, I was able to reproduce it. Would fix it.

d3xter666 commented 3 years ago

Hi @antonborisoff ,

The issue has been fixed with the following commit: https://github.com/SAP/openui5/commit/86ae04614d0ea53a715ce6138d0b80dde88306d5

Cheers

antonborisoff commented 3 years ago

Hi @d3xter666. Thanks a lot. Could you, please, downport it to SAPUI5 1.71.28 or 1.71.33?

d3xter666 commented 3 years ago

Hi @antonborisoff ,

It's been downported with the following commit: https://github.com/SAP/openui5/commit/6d47768703de97f0272f702055b11d71f02a7479

Cheers