Defect #32063

'use strict' in inlineAutoComplete causes error on Poltergeist

Added by Tatsuya Saito about 1 month ago. Updated 12 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Tatsuya Saito% Done:

0%

Category:-
Target version:-
Resolution:Wont fix Affected version:

Description

'use strict' in inlineAutoComplete causes error on Poltergeist.
r18444 with #31989 implemented inlineAutoComplete method in application.js.
After that, integration tests of my plugin have following errors.

Error:
Capybara::Poltergeist::JavascriptError: One or more errors were raised in the Javascript code on the page. If you don't care about these errors, you can ignore them by setting js_errors: false in your Poltergeist configuration (see documentation for details).

I attach a patch to fix it.

patch.diff Magnifier (1.02 KB) Tatsuya Saito, 2019-09-15 09:57

History

#1 Updated by Go MAEDA about 1 month ago

I am not good at JavaScript, so I am not sure why the code in the core should be changed in order to fix the error raised in a plugin. Is the code added in r18444 wrong?

#2 Updated by Tatsuya Saito about 1 month ago

Go MAEDA wrote:

I am not good at JavaScript, so I am not sure why the code in the core should be changed in order to fix the error raised in a plugin. Is the code added in r18444 wrong?

const statement in JavaScript was defined from ES6, but poltergeist(phantomJS) does not support ES6 and causes the error with use strict.
https://www.ecma-international.org/ecma-262/6.0/#sec-let-and-const-declarations
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const

r18444 added inlineAutoComplete method in application.js.
This method has const in use strict.
https://www.redmine.org/projects/redmine/repository/revisions/18444/diff/trunk/public/javascripts/application.js

I think old browsers without supporting ES6 may have same error (sorry, I don't check them).
So I think this problem should be fixed by core code.
If that old browsers are not supported, this problem is my plugin test, so should reject this issue.

I'm not good JavaScript too. Sorry if there is a mistake.

#3 Updated by Go MAEDA about 1 month ago

  • Assignee set to Marius BALTEANU

I understand. Thank you for the explanation.

Marius, please check the attached patch. Is it OK with you?

#4 Updated by Marius BALTEANU 27 days ago

  • Status changed from New to Needs feedback
  • Assignee changed from Marius BALTEANU to Tatsuya Saito

I don’t think we should write old JS syntax only to make the code compatible with PhantomJS which is no longer under active development. Also, Redmine is using Selenium with Chrome driver for system tests starting with version 4.

Regarding old browsers, I would be very happy to change the code if is not compatible with an old browser which is still supported by Redmine. Do you know one? From my tests, there is no problem in IE11.

#5 Updated by Tatsuya Saito 26 days ago

Marius BALTEANU,
Thank you for explanation, and I agree your opinion.
I don't know other browser with the problem. I think this issue shall be rejected.

#6 Updated by Go MAEDA 12 days ago

  • Status changed from Needs feedback to Closed
  • Resolution set to Wont fix

Also available in: Atom PDF