Skip to content

Checkout page hangs #9984

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

Closed
angelo983 opened this issue Jun 19, 2017 · 19 comments
Closed

Checkout page hangs #9984

angelo983 opened this issue Jun 19, 2017 · 19 comments
Labels
bug report Component: Catalog Fixed in 2.1.x The issue has been fixed in 2.1 release line Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed

Comments

@angelo983
Copy link
Member

angelo983 commented Jun 19, 2017

Wrong evaluating check of position param (Position at which item should be inserted.)
let hang Checkout Page

Preconditions

  1. Centos 6.7 Final
  2. Plesk 12.5 (Apache 2.2.15)
  3. Magento 2.1.7
  4. PHP 5.6
  5. Existing set of configurable products and linked simple products

Steps to reproduce

  1. Put in cart at least one simple product from a configurable product page
  2. Go to checkout page

Expected result

  1. Checkout page displays steps to complete payment.

Actual result

  1. After waiting a bit, the body of the page rests blank
  2. in console appears a javascript error: element position is null

Fixed Issues (if relevant)

lib/web/mage/utils/arrays.js
Line 105
from
if (typeof position === 'undefined') {
to
if (typeof position === 'undefined' || position == null) {

Manual testing scenarios

May happens while buying simple products of configurable products

@smoskaluk
Copy link

@angelo983 We cannot reproduce this issue as described. Please provide the detailed steps we must follow to reproduce this issue. In addition, identify the OS and web server you are running, the versions of MySQL, and any other information needed to reproduce your issue.

@angelo983
Copy link
Member Author

angelo983 commented Jun 23, 2017

I found a workaround for me by seeing this evidence in lib/web/mage/utils/arrays.js
Line 105 to 129

        if (typeof position === 'undefined') { // line 105 - fixed  issue above
            position = -1;
        } else if (typeof position === 'string') {
            position = isNaN(+position) ? position : +position;
        }

        newIndex = position;

        if (~currentIndex) {
            target = container.splice(currentIndex, 1)[0];

            if (typeof item === 'string') {
                item = target;
            }
        }

        if (typeof position !== 'number') {
            target = position.after || position.before || position; // line 122

            newIndex = getIndex(target, container);

            if (~newIndex && (position.after || newIndex >= currentIndex)) {
                newIndex++;
            }
        }

Browser console gave me error in line 122 because position value is NULL and this value is not managed in the script

@smoskaluk
Copy link

smoskaluk commented Jun 29, 2017

@angelo983 May you please give more detailed steps for reproducing this issue?

@angelo983
Copy link
Member Author

No need to reproduce, there is a evidence of faulty variable check as I stated in previous post

@magento-engcom-team
Copy link
Contributor

Hi @angelo983
thank you for research. Could you please submit pull request with your fix?

@magento-engcom-team magento-engcom-team added Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed and removed Progress: needs update labels Sep 13, 2017
@angelo983
Copy link
Member Author

Hello, I tried to pull request months ago without success.
I tried with base branch 2.1develop, have to choose another one?
This is the only change to apply:
lib/web/mage/utils/arrays.js
Line 105
from
if (typeof position === 'undefined') {
to
if (typeof position === 'undefined' || position == null) {

@magento-engcom-team
Copy link
Contributor

Hi @angelo983
You created the PR incorrectly #9966
Please, fork https://github.com/magento/magento2
Create new branch in your fork based on 2.1-develop branch
Apply the fix and commit to your new branch
Create pull request from your new branch to https://github.com/magento/magento2 2.1-develop branch

@magento-engcom-team
Copy link
Contributor

@angelo983 thank you for your report.
We were not able to reproduce this issue by following the steps you provided. Please provide more details regarding your environment, or try to reproduce this issue on a clean installation or latest release.

@angelo983
Copy link
Member Author

angelo983 commented Sep 20, 2017

You have not to reproduce this issue because of the nature of the issue itself, just put a developer to bring attention to this code

in lib/web/mage/utils/arrays.js
Line 105 to 129

    if (typeof position === 'undefined') { // line 105 - fixed issue transcribed below
        position = -1;
    } else if (typeof position === 'string') {
        position = isNaN(+position) ? position : +position;
    }

    newIndex = position;

    if (~currentIndex) {
        target = container.splice(currentIndex, 1)[0];

        if (typeof item === 'string') {
            item = target;
        }
    }

    if (typeof position !== 'number') {
        target = position.after || position.before || position; // line 122

        newIndex = getIndex(target, container);

        if (~newIndex && (position.after || newIndex >= currentIndex)) {
            newIndex++;
        }
    }

Error in line 122 because position value is NULL and this value is not managed in the script

This is the only change to apply:
lib/web/mage/utils/arrays.js
Line 105
from
if (typeof position === 'undefined') {
to
if (typeof position === 'undefined' || position == null) {

This is a EVIDENCE.

ps I tried anyway to fork and pull request again but forking hangs in "fetching latest commit..."

pps finally fork and pull request went on
#10975

@orlangur
Copy link
Contributor

@angelo983 I believe a problem is in which particular scenario position become equal to null. Error in console does not mean this is the right place for the fix.

@angelo983
Copy link
Member Author

@orlangur I believe instead that the programmer who wrote that line of code made the one mistake as expressed in the link https://stackoverflow.com/a/5076962 (difference between null and undefined in JavaScript).
If you are not convinced of this, state in what scenario the position should be of undefined type since that case is handled in the code
Summary:

Why YES?
if (typeof position === 'undefined') {

(My proposal)
if (typeof position === 'undefined' || position == null) {

@orlangur
Copy link
Contributor

@angelo983 I understand the difference between undefined and null in JavaScript and obviously such patch will work.

What I'm asking is whether position === null is even valid? Please provide some scenario when it happens, should be pretty easy.

@angelo983
Copy link
Member Author

@orlangur A scenario where this happens? Mine
Should be pretty easy too that you provide some scenario where typeof position === 'undefined'
But if you don't find it easy, I'll let you access my test platform. Let me know

@orlangur
Copy link
Contributor

May happens while buying simple products of configurable products

Does it happen from time to time?

Based on

We were not able to reproduce this issue by following the steps you provided. Please provide more details regarding your environment, or try to reproduce this issue on a clean installation or latest release.

it looks like not so easy to reproduce...

@angelo983
Copy link
Member Author

angelo983 commented Sep 25, 2017

In my scenario happens every time, my magento platform is at version 2.1.9 and started from 2.0.4
2.0.4 -> 2.0.7 -> 2.1.0 -> all subversions -> 2.1.9
the problem appeared from 2.1.4

@orlangur
Copy link
Contributor

@magento-engcom-team could you please recheck this issue taking into account the information provided?

@magento-engcom-team
Copy link
Contributor

PR has been merged to 2.1-develop branch. Closing the issue

@magento-engcom-team magento-engcom-team added Fixed in 2.1.x The issue has been fixed in 2.1 release line Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch labels Oct 18, 2017
@magento-engcom-team
Copy link
Contributor

@angelo983, thank you for your report.
We were not able to reproduce this issue by following the steps you provided. If you'd like to update it, please reopen the issue.
We tested the issue on 2.3.0-dev, 2.2.0, 2.1.9

@muckee
Copy link

muckee commented Aug 14, 2018

@orlangur the magento team is unable to reproduce the majority of commonly-occuring issues. That doesn't mean they don't happen often. I've had this problem with multiple installations of multiple versions of Magento 2. Currently experiencing it with Magento 2.2.5.

As I keep suggesting, Magento should release a guide on how to build a system the same as their ineffable 'vanilla' instances... because they seem to work flawlessly, but following Magento's guidance on server setup causes most people lots of problems.

Would save a lot of time scrolling through silly arguments on github...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Catalog Fixed in 2.1.x The issue has been fixed in 2.1 release line Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed
Projects
None yet
Development

No branches or pull requests

6 participants