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

Cacti 1.0 Does not Support jQueryUI 1.12.x #186

Closed
cigamit opened this issue Jan 10, 2017 · 23 comments
Closed

Cacti 1.0 Does not Support jQueryUI 1.12.x #186

cigamit opened this issue Jan 10, 2017 · 23 comments
Labels
enhancement General tag for an enhancement
Milestone

Comments

@cigamit
Copy link
Member

cigamit commented Jan 10, 2017

This can not be supported at this time due to the magnitude of the changes between CSS set's from jQueryUI 1.11.4 and jQueryUI 1.12.x.

@cigamit cigamit added this to the Cacti Release 1.02 milestone Jan 10, 2017
@cigamit cigamit added the enhancement General tag for an enhancement label Jan 10, 2017
@paulgevers
Copy link
Contributor

paulgevers commented Feb 17, 2017

Just so you are aware, this is a blocking issue for inclusion of cacti 1.0.x in Debian. Either you or I need to fix this if cacti is to be upgraded in Debian as it is unacceptable for Debian to use the embedded versions of the JavaScript libraries if they are available in Debian.

Can you explain where (which group of cacti files) cacti is so heavily tied to the jQueryUI CSS, such that I may have a shot at working on this?

@cigamit
Copy link
Member Author

cigamit commented Feb 18, 2017

Paul, Understand. jQueryUI choose to add many pseudo-classes in version 1.12. This broke all themes. It's more than a bug fix to remedy, but we will look into it. They should have made jQueryUI 1.12 a major version change. But unfortunately, they kept it a minor point release.

@paulgevers
Copy link
Contributor

@cigamit Could you point me at one location where I can see this in action in Cacti such that I can understand and potentially help?

@cigamit
Copy link
Member Author

cigamit commented Feb 19, 2017

Paul, the updates would be in the four theme directories. The files that need the new pseudo-classes are /usr/share/cacti/site/include/themes/*/jquery-ui.css.

@paulgevers
Copy link
Contributor

@cigamit These files also come from jQueryUI. But I see these files are (heavily?) modified in Cacti. You'll probably find this an extremely annoying question, but would it be possible (I mostly mean technically) to put the cacti changes in a separate file and load that after the default jQueuryUI one, to overload the default? That way it is easier to upgrade the jQueryUI files (which in Debian wouldn't come from Cacti anyways).

@cigamit
Copy link
Member Author

cigamit commented Feb 19, 2017

I suspect that they are actually not heavily edited, but that between jQueryUI 1.11 and jQueryUI 1.12 the themeroller CSS changed heavily. I agree with your point though.

@paulgevers
Copy link
Contributor

@cigamit would you accept patches that load main.css after all other style sheets and have the Cacti changes to the embedded style sheets defined in that main.css? Would that work as I expect it to work, i.e. that would overload the value in the earlier style sheet and thus enables Cacti to work with system jQueryUI?

@cigamit
Copy link
Member Author

cigamit commented Feb 23, 2017

I am making this first change as a part of Issue #329 commit

cigamit added a commit that referenced this issue Feb 23, 2017
Also first step in resolving issue #186
@cigamit cigamit removed this from the Cacti Release 1.0.3 milestone Mar 19, 2017
@paulgevers
Copy link
Contributor

paulgevers commented Apr 11, 2017

@cigamit It this close to what you would have in mind for include/themes/classic/main.css? The current implementation only tries to mimic the behavior of the current jquery-ui.css, i.e. it contains the delta of that file since commti d825504.

There are a couple of things I have to figure out:

  1. what the proper thing to do is for a change from "background color url()" to background-color color". Probably just go to "background color" as that is supposed to reset the other attributes.
  2. how to unset a variable, in one location there is a removal of the color (in .ui-state-active a"
  3. see how this needs to be changed for jQueryUI 1.12, I now think I see what you meant: the section with interactions looks now more detailed out.

main.css.txt

@paulgevers
Copy link
Contributor

@cigamit, it looks like the smoothness theme in 1.12 is already what you want for the modern theme in cacti. As far as I checked now, > 90% of the delta's introduced in cacti are already the default in jquery-ui 1.12 smoothness.

@ronytomen ronytomen added this to the Cacti Release 1.2 milestone May 29, 2017
@ronytomen ronytomen modified the milestones: Cacti Release 1.2, Cacti Release 1.3 Dec 22, 2017
@cigamit
Copy link
Member Author

cigamit commented Apr 23, 2018

Okay, first commit done. A few more to go to get everything straitened out.

cigamit added a commit that referenced this issue Apr 24, 2018
Cacti 1.x does not support jQueryUI 1.12.x.  This is the first of likely
a few commits to introduce jQueryUI 1.12.x into the core of Cacti.
cigamit added a commit that referenced this issue Apr 24, 2018
Changed Classic to smoothness and fixed up other oddities.  Should be
all set now.
@cigamit
Copy link
Member Author

cigamit commented Apr 24, 2018

Should be fully implemented now.

@cigamit cigamit closed this as completed Apr 24, 2018
@paulgevers
Copy link
Contributor

paulgevers commented Apr 24, 2018

@cigamit, just checking. In the history of this bug, we discussed that the jquery-ui.css files would be taken from upstream and that all Cacti changes would be in the main.css files. In your commit f9a490e I do see that you changed the classic/jquery-ui.css file. I guess that is a change in the upstream file. If I am correct, can't that change be carried (by overloading) in the classic/main.css file?

And much thanks for finally implementing this.

@cigamit
Copy link
Member Author

cigamit commented Apr 24, 2018

Paul,

Here is the process:

  1. Take the header from the themes jquery-ui.css and post it into the jQueryUI Theme roller page. The URL there is the base of the Cacti theme.
  2. Request to download everything using the 1.12.x schema. Most right now are smoothness, though Dark and Modern are 'Custom'.
  3. Copy all the required stuff to the appropriate location within Cacti.
  4. We are overload everything. There are no modifications to the jquery-ui.css. They are stock.

Note: Under the Classic theme there were a number of unused image files that were removed. There were like 2 per each of the other themes plus some other stuff that somehow got committed.
Note: I changed the Classic theme to use smoothness. That's why you are seeing all the changes there.
Note: There will be another update this morning. There are going to be two *.php files modified to make this work. Make sure you track those changes too.

I think that explains it all.

@paulgevers
Copy link
Contributor

@cigamit thanks for the explanation.

The second note explained my question I guess.

I now have a different question. In Debian I strip the jquery-ui code from Cacti and use the distribution version of jquery-ui (it is a separate package). With changes in Cacti files overloading the default I am perfectly happy, but I wonder what is going to happen (it probably already is happening with the Debian package of Cacti) with the Dark and Modern themes, as they have code that Debian doesn't know about. I don't have the answer (maybe there isn't a straight forward one), but maybe you have some thoughts?

cigamit added a commit that referenced this issue Apr 24, 2018
The autocomplete dropdown was not funcational and had the wrong styling.
@cigamit
Copy link
Member Author

cigamit commented Apr 24, 2018

Good point. That one is tricky. Debian needs to support custom themes. I would suggest that you leave things in their current develop places as Cacti will always be dependent, as well as other OSS packages, on the release of jQueryUI. Having jquery-ui.js somewhere else does not bother me too much, but the theme files should remain inside the Cacti distribution. A purist might say that if they are to be customized by users, they should belong in their own folder separate from Cacti. That point is well taken. We have the same issue with Plugins.

You should take this up with the Debian team. JQueryUI breaks a lot of things in the minor point releases. For example, they introduced pseudo-classes between 1.11 and 1.12 and that was catastrophic to Cacti.

It turned out to be less work than I thought making it compatible again. But I suspect this is going to happen again in the future.

The last point, is that we are trying, though it may not appear so, to allow users to download and install Plugins, Packages, and Themes from the internet directly (github more specifically). It's on our to do list, but we are dysfunctional due to our day jobs.

cigamit added a commit that referenced this issue Apr 24, 2018
Remove width on the wrapping span.  It's no longer required.
@paulgevers
Copy link
Contributor

Then you'll be not too pleased to learn what I do in Debian. For example look at the tree of the classic theme:

paul@testavoira ~ $ tree /usr/share/cacti/site/include/themes/classic/
/usr/share/cacti/site/include/themes/classic/
├── c3.css -> ../../../../../javascript/c3/c3.min.css
├── default
│   ├── 32px.png -> ../../../../../../javascript/jstree/themes/default/32px.png
│   ├── 40px.png -> ../../../../../../javascript/jstree/themes/default/40px.png
│   ├── index.php
│   ├── style.css -> ../../../../../../javascript/jstree/themes/default/style.css
│   └── throbber.gif -> ../../../../../../javascript/jstree/themes/default/throbber.gif
├── images
│   ├── 128
│   │   ├── bar-alpha.png -> ../../../../../../../javascript/libjs-jquery-colorpicker/images/128/bar-alpha.png
│   │   ├── bar.png -> ../../../../../../../javascript/libjs-jquery-colorpicker/images/128/bar.png
│   │   └── map.png -> ../../../../../../../javascript/libjs-jquery-colorpicker/images/128/map.png
│   ├── bar-alpha.png -> ../../../../../../javascript/libjs-jquery-colorpicker/images/bar-alpha.png
│   ├── bar-opacity.png -> ../../../../../../javascript/libjs-jquery-colorpicker/images/bar-opacity.png
│   ├── bar.png -> ../../../../../../javascript/libjs-jquery-colorpicker/images/bar.png
│   ├── bar-pointer.png -> ../../../../../../javascript/libjs-jquery-colorpicker/images/bar-pointer.png
│   ├── cacti_logo.gif
│   ├── cacti_logo.svg
│   ├── favicon.ico
│   ├── index.php
│   ├── map-opacity.png -> ../../../../../../javascript/libjs-jquery-colorpicker/images/map-opacity.png
│   ├── map.png -> ../../../../../../javascript/libjs-jquery-colorpicker/images/map.png
│   ├── map-pointer.png -> ../../../../../../javascript/libjs-jquery-colorpicker/images/map-pointer.png
│   ├── preview-opacity.png -> ../../../../../../javascript/libjs-jquery-colorpicker/images/preview-opacity.png
│   ├── tab_list.gif
│   ├── tab_preview.gif
│   ├── tab_tree.gif
│   ├── ui-bg_glass_55_fcf0ba_1x400.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-bg_glass_55_fcf0ba_1x400.png
│   ├── ui-bg_gloss-wave_100_ece8da_500x100.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-bg_gloss-wave_100_ece8da_500x100.png
│   ├── ui-bg_highlight-hard_100_f5f3e5_1x100.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-bg_highlight-hard_100_f5f3e5_1x100.png
│   ├── ui-bg_highlight-hard_100_fafaf4_1x100.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-bg_highlight-hard_100_fafaf4_1x100.png
│   ├── ui-bg_highlight-hard_15_459e00_1x100.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-bg_highlight-hard_15_459e00_1x100.png
│   ├── ui-bg_highlight-hard_95_cccccc_1x100.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-bg_highlight-hard_95_cccccc_1x100.png
│   ├── ui-bg_highlight-soft_25_67b021_1x100.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-bg_highlight-soft_25_67b021_1x100.png
│   ├── ui-bg_highlight-soft_95_ffedad_1x100.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-bg_highlight-soft_95_ffedad_1x100.png
│   ├── ui-bg_inset-soft_15_2b2922_1x100.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-bg_inset-soft_15_2b2922_1x100.png
│   ├── ui-colorpicker.png -> ../../../../../../javascript/libjs-jquery-colorpicker/images/ui-colorpicker.png
│   ├── ui-icons_222222_256x240.png -> ../../../../../../javascript/jquery-ui-themes/smoothness/images/ui-icons_222222_256x240.png
│   ├── ui-icons_808080_256x240.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-icons_808080_256x240.png
│   ├── ui-icons_847e71_256x240.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-icons_847e71_256x240.png
│   ├── ui-icons_8DC262_256x240.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-icons_8DC262_256x240.png
│   ├── ui-icons_cd0a0a_256x240.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-icons_cd0a0a_256x240.png
│   ├── ui-icons_eeeeee_256x240.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-icons_eeeeee_256x240.png
│   └── ui-icons_ffffff_256x240.png -> ../../../../../../javascript/jquery-ui-themes/south-street/images/ui-icons_ffffff_256x240.png
├── index.php
├── jquery.colorpicker.css -> ../../../../../javascript/libjs-jquery-colorpicker/jquery.colorpicker.css
├── jquery.multiselect.css
├── jquery.timepicker.css -> ../../../../../javascript/jquery-timepicker/jquery-ui-timepicker-addon.css
├── jquery-ui.css -> ../../../../../javascript/jquery-ui-themes/south-street/jquery-ui.css
├── jquery-ui.min.css -> ../../../../../javascript/jquery-ui-themes/south-street/jquery-ui.min.css
├── jquery.zoom.css
├── main.css
├── main.js
├── pace.css
├── rrdtheme.php
└── theme.css -> ../../../../../javascript/jquery-ui-themes/south-street/theme.css

3 directories, 53 files

@paulgevers
Copy link
Contributor

And the location in for local changes compared to /usr/share on Debian is in /usr/local/share. But if Cacti would support that it would mean logic to see if the local admin added anything there and otherwise fall back to the default. The point for distribution like Debian is that all files except those under /etc/ are allowed (and typically automatically) updated upon package upgrade.

cigamit added a commit that referenced this issue Apr 24, 2018
@cigamit
Copy link
Member Author

cigamit commented Apr 24, 2018

I think we could squeak things into our config.php for locations of things like themes for sure. Let me give it some thought.

@cigamit
Copy link
Member Author

cigamit commented Apr 24, 2018

We now have a common header function, so the number of places we would have to make changes is pretty low.

@netniV
Copy link
Member

netniV commented Apr 24, 2018

Some other places you may find would have issues with that is again plugins and code which uses $config['base_path'] as the starting point for building a hard coded path.

@cigamit
Copy link
Member Author

cigamit commented Apr 25, 2018

I guess the other option would to be to support the concept of theme-packs and then some relatively dynamic directory that holds the extracted theme files, for each of the users sessions. That temporary location would be pruned as sessions come and go.

Theme packs could include exploits though.

Paul, what do you think about this idea. That way, you could install/uninstall theme packs anywhere on the OS per path and store that path in the database. Again, it's a departure of the OS vendor controlling such things, but maybe that is best.

@paulgevers
Copy link
Contributor

@cigamit If it is optional than I am fine (on my users behalf) The user could opt to install the distributions version and stay with that, or install a theme package from anywhere on the Internet and use that. In the latter case, the risks are clearly higher for exploits than in the former.

WordPress in Debian does similar thinks if I am correct (with plugins; not sure how/if this is supported by WordPress upstream). Some plugins are distributed by Debian and can be installed in the regular way. If the WordPress user/admin wants a newer version (or isn't admin on the system) he can install the plugins in the WordPress way.

If this is then the only way, I don't really like it.

@ronytomen ronytomen modified the milestones: Cacti 1.3, Cacti 1.2 Sep 21, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement General tag for an enhancement
Projects
None yet
Development

No branches or pull requests

4 participants