Upgraded ExtJS to 1.1.1 #2473

justinbeaty · 2022-08-22T18:27:36Z

Description (*)

Thanks to @fballiano for pointing this out on discord. ext-tree.js is version 1.0.1 in our source. I've updated to 1.1.1 which was the last 1.x release (apparently 2.x was BC-breaking). I also updated the bundled resources (CSS and images)

It turns out ext-tree.js is not just the tree component from ExtJS, but a custom build of ExtJS. I had to recreate this from the ext-1.1.1.zip archive.

Source is available here: https://web.archive.org/web/20130401000000*/http://dev.sencha.com/deploy/ext-1.1.1.zip

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

There are the files fix-defer-before.js and fix-defer.js. The former looks like an MSIE7 fix. The latter I'm not sure, but possibly some polyfill or browser fix. It's probably worth looking into what those both do.

There is also the ext/css dir which contains files that look like they're from ExtJS 2.0. I'm not sure what they're doing there and I don't see usage of them.

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)
Add yourself to contributors list

justinbeaty · 2022-08-22T20:06:01Z

In an attempt to reduce the size of the bundle, here's an updated build script. I am basically seeing what can be removed from the files array. 224KB -> 168KB (a 25% reduction).

#!/bin/bash

files=(
  source/core/Ext.js
  source/adapter/prototype-bridge.js
  source/core/DomHelper.js
  # source/core/Template.js
  # source/core/DomQuery.js
  source/util/Observable.js
  source/core/EventManager.js
  source/core/Element.js
  source/core/Fx.js
  # source/core/CompositeElement.js
  source/core/UpdateManager.js
  # source/util/DelayedTask.js
  # source/util/TaskMgr.js
  source/util/MixedCollection.js
  source/util/JSON.js
  # source/util/Format.js
  # source/util/CSS.js
  # source/util/ClickRepeater.js
  # source/util/KeyNav.js
  # source/util/KeyMap.js
  # source/util/TextMetrics.js
  source/dd/DDCore.js
  source/dd/ScrollManager.js
  source/dd/Registry.js
  source/dd/StatusProxy.js
  source/dd/DragSource.js
  source/dd/DropTarget.js
  source/dd/DragZone.js
  source/dd/DropZone.js
  # source/data/SortTypes.js
  # source/data/Record.js
  # source/data/Store.js
  source/data/Connection.js
  # source/data/DataField.js
  # source/data/DataReader.js
  # source/data/DataProxy.js
  # source/data/MemoryProxy.js
  # source/data/HttpProxy.js
  # source/data/ScriptTagProxy.js
  # source/data/JsonReader.js
  # source/data/ArrayReader.js
  source/data/Tree.js
  source/widgets/Component.js
  source/widgets/BoxComponent.js
  source/widgets/Layer.js
  source/widgets/Shadow.js
  source/widgets/Editor.js
  source/widgets/tree/TreePanel.js
  source/widgets/tree/TreeSelectionModel.js
  source/widgets/tree/TreeNode.js
  source/widgets/tree/AsyncTreeNode.js
  source/widgets/tree/TreeNodeUI.js
  source/widgets/tree/TreeLoader.js
  source/widgets/tree/TreeFilter.js
  source/widgets/tree/TreeSorter.js
  source/widgets/tree/TreeDropZone.js
  source/widgets/tree/TreeDragZone.js
  source/widgets/tree/TreeEditor.js
  source/widgets/form/Field.js
  source/widgets/form/TextField.js
  )


# License Header
printf '/**\n' > ext-tree.js
printf ' * Ext JS Library 1.1.1\n' >> ext-tree.js
tail -n +2 LICENSE.txt | sed -e 's/^/ \* /' >> ext-tree.js
printf '\n */\n' >> ext-tree.js

# Concat files
for file in "${files[@]}"; do
    tail -n +8 $file >> ext-tree.js
    printf '\n\n\n' >> ext-tree.js
done


# License Header
printf '/**\n' > ext-tree.min.js
printf ' * Ext JS Library 1.1.1\n' >> ext-tree.min.js
tail -n +2 LICENSE.txt | sed -e 's/^/ \* /' >> ext-tree.min.js
printf '\n */\n' >> ext-tree.min.js

# Concat files
for file in "${files[@]}"; do
    file=${file//source/build}
    file=${file//\.js/-min\.js}
    tail -n +8 $file >> ext-tree.min.js
    printf '\n\n\n' >> ext-tree.min.js
done


dos2unix ext-tree.js
dos2unix ext-tree.min.js

ADDISON74 · 2022-08-22T21:11:57Z

In the repository can be the normal and the reduced size. I don't agree with only one reduced version. Such a format is supposed to be final, for production, free of issues and we all know that this is impossible. To check the code we have to make another step but when I propose a PR in a minified version it will be difficult for others to follow. Classic coding is preferred for the project (my opinion).

ADDISON74 · 2022-08-22T21:17:46Z

I did not find any references in the code related to files:

fix-defer-before.js (this one has the code commented and with references to IE7 and IE8) => it could be deleted.
fix-defer.js (this one seems to defer something) => I don't know what is the impact if it is deleted.

All files in CSS directory are from Ext JS Library 2.0.

justinbeaty · 2022-08-22T21:22:57Z

I see fix-defer-before.js included here (Edit: you're right, the file was commented out :P)

$ grep -R 'fix-defer-before.js' ~/magento-public/htdocs/app/
/home/www-data/magento-public/htdocs/app/design/adminhtml/default/default/layout/main.xml:                <action method="addItem"><type>js</type><name>extjs/fix-defer-before.js</name><params/><if/><condition>can_load_ext_js</condition></action>
/home/www-data/magento-public/htdocs/app/design/adminhtml/default/default/layout/oauth.xml:            <action method="removeItem"><type>js</type><name>extjs/fix-defer-before.js</name></action>

I'm not sure yet either the reason for fix-defer.js, but I'll get to the bottom of it.

In the repository can be the normal and the reduced size. I don't agree with only one reduced versions. Such a format is supposed to be final, for production, free of issues and we all know that this is impossible. To check the code we have to make another step but when I propose a PR in a minified version it will be difficult for others to follow. Classic coding is preferred for the project (my opinion).

Normally I agree. In this case I don't want to include the entire ExtJS library since we only use a very small subset, but I should be able to include a non-minified source of each file I'm concating together.

lgtm-com · 2022-08-22T22:20:18Z

This pull request fixes 1 alert when merging d0b9975 into 8297316 - view on LGTM.com

fixed alerts:

1 for Conditional comments

lgtm-com · 2022-08-22T22:48:51Z

This pull request fixes 1 alert when merging 03ab001 into 8297316 - view on LGTM.com

fixed alerts:

1 for Conditional comments

lgtm-com · 2022-08-22T22:59:11Z

This pull request fixes 1 alert when merging 22950cc into 8297316 - view on LGTM.com

fixed alerts:

1 for Conditional comments

justinbeaty · 2022-08-22T23:02:53Z

I removed a lot of files that I don't think are needed at all. Maybe too much, but I left only files that are used by Mage as far as I can tell.

There's a slight chance that some 3rd party extension used some of these JS components, but I really doubt it. Because if they did they would have been limited to the ones Magento team chose to bundle into ext-tree.js. It's more likely a module developer would have bundled their own ExtJS files into their module. Plus ExtJs 1.x is from 2007, and was replaced by ExtJS 2.x around the same time too.

In any case, I say to get rid of all this old JS. One day when we want to modernize the JS stack, we will be thankful that there is less crap to go through.

fballiano · 2022-08-28T19:46:27Z

do you think this is ready for test?

justinbeaty · 2022-08-28T19:48:44Z

@fballiano yes I think it’s ready. I forgot it was marked draft, I’ve been out of town last few days.

lgtm-com · 2022-08-28T19:49:46Z

This pull request fixes 1 alert when merging e58a2d4 into b370350 - view on LGTM.com

fixed alerts:

1 for Conditional comments

fballiano

tested both categories and products mask work fine.

lgtm-com · 2022-09-27T20:14:16Z

This pull request fixes 1 alert when merging 7070b3d into 00748ce - view on LGTM.com

fixed alerts:

1 for Conditional comments

elidrissidev

All good after testing the mentioned pages

fballiano · 2022-12-27T14:39:06Z

fixed conflict, could you re-test just to be sure?

justinbeaty · 2023-01-14T14:45:17Z

fixed conflict, could you re-test just to be sure?

Hi Sorry, I missed this comment earlier. I will check it now, but do we still want to target 1.9.4.x with this PR? Not sure if we've changed thoughts on this.

Edit: I bring up the question of target branch because of my comment above:

There's a slight chance that some 3rd party extension used some of these JS components, but I really doubt it. Because if they did they would have been limited to the ones Magento team chose to bundle into ext-tree.js. It's more likely a module developer would have bundled their own ExtJS files into their module. Plus ExtJs 1.x is from 2007, and was replaced by ExtJS 2.x around the same time too.

Just in case that some module breaks on the extjs upgrade, it may be better to leave out of 1.9.4.x and target 20.0.

fballiano · 2023-01-14T15:33:06Z

well, we should rebase on v20, but if we do:

justinbeaty · 2023-04-02T18:54:18Z

@fballiano I don't recall off the top of my head, but I'll take a look a bit later today or tomorrow morning at the latest. I'll also rebase to v20 while I'm at it.

justinbeaty · 2023-04-03T16:23:56Z

@fballiano I believe I had just manually removed all things not related to ext-tree from the CSS file. The reason I missed that class is that it's not from extjs, but from ext-tree-checkbox.js, which is an plugin for extjs that came from somewhere (there's no copyright or license info in that file)...

I've re-added that class & rebased to 20.0

fballiano · 2023-04-03T19:15:46Z

that came from somewhere (there's no copyright or license info in that file)...

😂😂

fballiano · 2023-04-03T19:23:31Z

@justinbeaty super, seems to be perfect now. I see a few missing classes (#loading for example) but who knows if they're really necessary...

justinbeaty · 2023-04-03T19:29:32Z

@fballiano yeah, I noticed those #loading styles too, and I don’t think they were used either. I’ll check it out momentarily, but if they were used, I can probably refactor it to use something more specific than just #loading.

fballiano · 2023-04-03T19:31:39Z

agree, let's merge and see if something comes out we'll fix it later :-)

justinbeaty · 2023-04-03T20:35:22Z

Yes, I don't see anything creating <div id="loading"> in extjs. Instead it creates:

indicatorText : '<div class="loading-indicator">Loading...</div>'

So I think we are all good.

ADDISON74 · 2023-04-03T20:51:21Z

With all the recent changes we should take a look into the browser console and do an audit. There is a small chance that there are still issues related to ZF1-Future and what was removed that we aren't aware.

justinbeaty · 2023-04-03T21:13:06Z

@ADDISON74 I checked all three pages mentioned in the initial comment with the browser console opened. No errors.

github-actions bot added the JavaScript Relates to js/* label Aug 22, 2022

github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Oauth Relates to Mage_Oauth Template : admin Relates to admin template labels Aug 22, 2022

fballiano force-pushed the topic-extjs branch from 22950cc to e58a2d4 Compare August 28, 2022 19:43

justinbeaty marked this pull request as ready for review August 28, 2022 19:49

fballiano previously approved these changes Aug 28, 2022

View reviewed changes

fballiano force-pushed the topic-extjs branch from e58a2d4 to 7070b3d Compare September 27, 2022 20:07

fballiano requested review from ADDISON74, kiatng and elidrissidev September 27, 2022 20:07

sreichel dismissed fballiano’s stale review via 7070b3d September 28, 2022 01:04

elidrissidev previously approved these changes Sep 29, 2022

View reviewed changes

fballiano dismissed elidrissidev’s stale review via f186541 December 27, 2022 14:37

fballiano changed the base branch from 1.9.4.x to 20.0 January 14, 2023 15:32

justinbeaty added 13 commits April 3, 2023 09:09

Upgrade extjs to 1.1.1

e9e6a8e

smaller js file

3f745b4

remove unused images

0919ea7

remove unused theme css files

b4ab543

Reduce extjs magento theme

085a0f1

remove css files from layout

da5e651

Remove ExtJs 2.0 css files

310d769

add core/fx

161d55c

add util/JSON

5290345

add note about fix-defer.js

6f5524d

add source file

c08f49b

Add source/data/Connection.js

a68da7a

add input.l-tcb

5166eac

justinbeaty force-pushed the topic-extjs branch from 2440950 to 5166eac Compare April 3, 2023 16:22

justinbeaty changed the base branch from 1.9.4.x to 20.0 April 3, 2023 16:22

fballiano approved these changes Apr 3, 2023

View reviewed changes

elidrissidev approved these changes Apr 3, 2023

View reviewed changes

fballiano changed the title ~~Upgrade ExtJS to 1.1.1~~ Upgraded ExtJS to 1.1.1 Apr 3, 2023

fballiano merged commit 6b66b31 into OpenMage:20.0 Apr 3, 2023

elidrissidev mentioned this pull request Jun 9, 2023

ownerTree.container is undefined for product categories #3312

Closed

justinbeaty mentioned this pull request Jun 9, 2023

Fix #3312 ExtJS Tree Checkbox Update #3314

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgraded ExtJS to 1.1.1 #2473

Upgraded ExtJS to 1.1.1 #2473

justinbeaty commented Aug 22, 2022 •

edited

Loading

justinbeaty commented Aug 22, 2022 •

edited

Loading

ADDISON74 commented Aug 22, 2022 •

edited

Loading

ADDISON74 commented Aug 22, 2022 •

edited

Loading

justinbeaty commented Aug 22, 2022 •

edited

Loading

lgtm-com bot commented Aug 22, 2022

lgtm-com bot commented Aug 22, 2022

lgtm-com bot commented Aug 22, 2022

justinbeaty commented Aug 22, 2022

fballiano commented Aug 28, 2022

justinbeaty commented Aug 28, 2022

lgtm-com bot commented Aug 28, 2022

fballiano left a comment

lgtm-com bot commented Sep 27, 2022

elidrissidev left a comment

fballiano commented Dec 27, 2022

justinbeaty commented Jan 14, 2023 •

edited

Loading

fballiano commented Jan 14, 2023

justinbeaty commented Apr 2, 2023

justinbeaty commented Apr 3, 2023

fballiano commented Apr 3, 2023

fballiano commented Apr 3, 2023

justinbeaty commented Apr 3, 2023

fballiano commented Apr 3, 2023

justinbeaty commented Apr 3, 2023

ADDISON74 commented Apr 3, 2023

justinbeaty commented Apr 3, 2023

Upgraded ExtJS to 1.1.1 #2473

Upgraded ExtJS to 1.1.1 #2473

Conversation

justinbeaty commented Aug 22, 2022 • edited Loading

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

justinbeaty commented Aug 22, 2022 • edited Loading

ADDISON74 commented Aug 22, 2022 • edited Loading

ADDISON74 commented Aug 22, 2022 • edited Loading

justinbeaty commented Aug 22, 2022 • edited Loading

lgtm-com bot commented Aug 22, 2022

lgtm-com bot commented Aug 22, 2022

lgtm-com bot commented Aug 22, 2022

justinbeaty commented Aug 22, 2022

fballiano commented Aug 28, 2022

justinbeaty commented Aug 28, 2022

lgtm-com bot commented Aug 28, 2022

fballiano left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Sep 27, 2022

elidrissidev left a comment

Choose a reason for hiding this comment

fballiano commented Dec 27, 2022

justinbeaty commented Jan 14, 2023 • edited Loading

fballiano commented Jan 14, 2023

justinbeaty commented Apr 2, 2023

justinbeaty commented Apr 3, 2023

fballiano commented Apr 3, 2023

fballiano commented Apr 3, 2023

justinbeaty commented Apr 3, 2023

fballiano commented Apr 3, 2023

justinbeaty commented Apr 3, 2023

ADDISON74 commented Apr 3, 2023

justinbeaty commented Apr 3, 2023

justinbeaty commented Aug 22, 2022 •

edited

Loading

justinbeaty commented Aug 22, 2022 •

edited

Loading

ADDISON74 commented Aug 22, 2022 •

edited

Loading

ADDISON74 commented Aug 22, 2022 •

edited

Loading

justinbeaty commented Aug 22, 2022 •

edited

Loading

justinbeaty commented Jan 14, 2023 •

edited

Loading