LibrePlan / libreplan

LibrePlan - Open Web Planning
https://www.libreplan.dev
GNU Affero General Public License v3.0
287 stars 170 forks source link

Zoom level drop down box not functional in Gantt diagram #1909

Open grypho opened 5 years ago

grypho commented 5 years ago

In the gantt diagrams there is a zoomlevel drop-down list (Year, Quarter, Month, Week, Day). At least in my used Browsers (recent FF, Chrome) nothing happens when I change the zoom level. (Maybe related to #372 )

Can anyone confirm that issue in the current git version?

grypho commented 5 years ago

Issue is not related to version of jQuery. Same behaviour with jQuery 3.x

grypho commented 5 years ago

After some investigation using the non-minified zk library it seems that a rendering operation of the drop-down box (select tag) unregisters the _zoomLevelChanged handler. So far I'm not sure why the component is rendered again and how the event handler can be re-attached (it's eleven years old code). I doubt this effect was introduced by an update of the zk library as our Planner does extend these components.

grypho commented 5 years ago

Update on this topic after several hours of debugging. It looks like the Java class (TimeTrackerComponent) which is linked to the TimeTracker UI component gets destructed for some reason.

(1) Initially when the site is built, an instance of TimeTrackerComponent is being created and registers an IZoomLevelChangedListener. That listener is called by TimeTracker.fireZoomChanged() after the compose process and sets the initial zoom to the right level. (all steps verified by additional profiling log entries).

(2) When I change the zoom level later by selecting another item in the drop-down box, the method TimeTracker.fireZoomChanged() which fires the event again. Within the method, the method zoomListeners.fireEvent is called which should iterate all registered handlers. In the begining of that method another method getActiveListeners is called to retrieve a list of active handlers. There all registered handlers are checked if they are null (=> they are removed from the handler list) or not null (=> they are returned by getActiveListeners). Sadly, the former registered TimeTrackerComponent callback is null at this point and is being removed from the list and never called again.

I do not have enough experience in Java but I think, the TimeTrackerComponent (or at least the event Handler) is being destructed by the garbage collector for some reason between (1) and (2). Any Ideas how to track it down?

grypho commented 5 years ago

This issue has the same cause as #406. Issue 406 adresses only the Advance assignment window but the issue also happens on every other window which uses the GanttPlanner component and where the zoom can be changed.

grypho commented 5 years ago

Update on this topic after some more hours of debugging, stack dumping and investigating.

On Aug 14 my assumption (2) was not correct at all. When a GanttPanel component (the charting area where Tasks, Dependencies, ... are being drawn) is instanciated, two instances which derive from TimeTrackerComponent will be created:

Both components do exist at the same time. For some reason (I think the garbage collector is involved) their registered callbacks IZoomLevelChangedListener (Constructor of TimeTrackerComponent) is destructed (maybe a combination of delta expression and WeakReference?). Nevertheless when I store all instances of TimeTrackerComponent in a List and call the method zoomChanged manually the behavior of the Gantt component is as expected and the zoom works fine.

I'll continue investigating the GC+Weak assumption and will provide a pull request afterwards.

j-arnott commented 3 years ago

In the plannerLayout.zul I had to change the zoom listbox code to:

<listbox id="listZoomLevels" mold="select" rows="1" model="${planner.zoomLevels}" onSelect="planner.setZoomLevel(self.selectedItem.value,1);" />

Not sure what the scroll left arg does.

tramseyer commented 3 years ago

In the plannerLayout.zul I had to change the zoom listbox code to:

<listbox id="listZoomLevels" mold="select" rows="1" model="${planner.zoomLevels}" onSelect="planner.setZoomLevel(self.selectedItem.value,1);" />

Not sure what the scroll left arg does.

Works for me as well.

kwoot commented 3 years ago

Can somebody make a PR that I can accept?

kwoot commented 3 years ago

PR merged. Would be nice if somebody can confirm it works, cause I am currently swamped with work. (sorry).

tramseyer commented 3 years ago

PR merged. Would be nice if somebody can confirm it works, cause I am currently swamped with work. (sorry).

Its work for me.