Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check to avoid js error #1380

Merged
merged 1 commit into from
Jul 31, 2021
Merged

Add check to avoid js error #1380

merged 1 commit into from
Jul 31, 2021

Conversation

luigifab
Copy link
Contributor

@luigifab luigifab commented Jan 8, 2021

Description

This PR add a check to avoid a javascript fatal error when I merge and minify all backend javascript files into one single file.

screen

OpenMage 20.0.5 / PHP 7.4.11 - 8.0.0

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

@colinmollenhour
Copy link
Member

Does the init happen later or should it be added to an event observer like onload?

@luigifab
Copy link
Contributor Author

luigifab commented Jan 9, 2021

Function _initWindowElements via initialize is already triggered on page load (at the end of the file).
Perhaps it is better to add the check here:

Event.observe(window, 'load',  function() {
    productConfigure = new ProductConfigure();
});

@kiatng kiatng added Component: Adminhtml Relates to Mage_Adminhtml JavaScript Relates to js/* labels Jan 10, 2021
@Flyingmana
Copy link
Contributor

in any case I would think it needs to wait for it to exist, and not just skipping it?
I think the current place is already ok to add the check, but maybe add a retry logic with setTimeout.

@luigifab
Copy link
Contributor Author

For my and standard usage, a setTimeout is useless. Because if product_composite_configure does not exist on page load, it will never exist.

@Flyingmana Flyingmana merged commit e9eff3b into OpenMage:1.9.4.x Jul 31, 2021
@github-actions
Copy link
Contributor

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
6 runs  +4  4 ✔️ +2  2 💤 +2  0 ❌ ±0 

Results for commit e9eff3b. ± Comparison against base commit 6270dc4.

@luigifab luigifab deleted the prevent-jserror branch July 31, 2021 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml JavaScript Relates to js/*
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants