Bug 26214 - MultiInput-Widgets informieren Form nicht über erfolgreiche Initialisierung
MultiInput-Widgets informieren Form nicht über erfolgreiche Initialisierung
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC (Generic)
UCS 3.0
Other Linux
: P3 normal (vote)
: UCS 3.1
Assigned To: Alexander Kläser
Dirk Wiesenthal
: interim-3
Depends on: 25630
Blocks: 26216 28400 28797
  Show dependency treegraph
 
Reported: 2012-02-22 13:41 CET by Sönke Schwardt-Krummrich
Modified: 2013-07-19 11:31 CEST (History)
5 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Usability
Max CVSS v3 score:
klaeser: Patch_Available+


Attachments
Teilfix mit Debugausgaben (5.12 KB, patch)
2012-02-22 13:41 CET, Sönke Schwardt-Krummrich
Details | Diff
Patch für umc-frontend (20.13 KB, patch)
2012-10-19 22:02 CEST, Alexander Kläser
Details | Diff
Patch für umc-module-udm (1.87 KB, patch)
2012-10-19 22:02 CEST, Alexander Kläser
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sönke Schwardt-Krummrich univentionstaff 2012-02-22 13:41:37 CET
Created attachment 4209 [details]
Teilfix mit Debugausgaben

Es senden nicht alle Widgets ein onValuesLoaded-Event. Betroffen sind
einige/alle MultiInput-Widgets, die dynamisch Daten nachladen.

Eine Teilimplementierung mit Debug-Ausgabe hängt als Patch an.

+++ This bug was initially created as a clone of Bug #25630 +++

Wenn Änderungen an einem Objekt vorgenommen werden und ohne Speicherung zur
Übersicht zurückgewechselt wird sollte eine Warnung kommen, die darauf
hinweist.
Comment 1 Sönke Schwardt-Krummrich univentionstaff 2012-02-22 14:05:50 CET
umc/widgets/Form.js geht davon aus, dass alle Widgets ein onValuesLoaded zurückgeben, wenn diese fertig gerendert sind und alle Werte geladen wurden.
In Bug 25630 wurden in diesem Moment die aktuellen Formularwerte gespeichert, um später z.B. die Differenz zu den Initialwerten ermitteln zu können.

Allerdings emittieren die MultiInput-Widgets kein onValuesLoaded, so dass bei der Verwendung von dynamischen Widgets (ComboBox) zu früh angenommen wurde, es wäre alles geladen. Die Kopie der Initialwerte war damit unvollständig.

Der Patch konnektiert sich in MultiInput an das onValuesLoaded aller Kindwidgets und fordert diese auf, ihre Werte zu laden. Ist das Event von allen Kindern eingetroffen, wird das Event von MultiInput selbst emittiert.

Der Patch behebt das Problem noch nicht vollständig, da nicht alle Kindwidgets ein Event senden (vermutlich immer dann, wenn dynamisch eine leere Liste nachgeladen wurde und somit praktisch kein Wert zu setzen war).
Comment 2 Stefan Gohmann univentionstaff 2012-07-17 17:09:46 CEST
UCS 3.1 will be the next release.
Comment 3 Alexander Kläser univentionstaff 2012-10-19 21:18:13 CEST
Beim Bearbeiten dieses Bugs ist folgender komischer Bug aufgetreten:

  http://bugs.dojotoolkit.org/ticket/16209

Als Workaround habe ich eine händisches Kopieren aller Konstruktor-Parameter in umc/widgets/LabelPane:constructor eingebaut. Der Fehler hatte zur Folge, dass in einigen Fällen die Sprach-Auswahlbox des Login-Dialogs nicht zu sehen war und das manchmal das Suchformular von UDM-Modulen nicht initialisiert war.
Comment 4 Alexander Kläser univentionstaff 2012-10-19 22:02:23 CEST
Created attachment 4727 [details]
Patch für umc-frontend
Comment 5 Alexander Kläser univentionstaff 2012-10-19 22:02:48 CEST
Created attachment 4728 [details]
Patch für umc-module-udm
Comment 6 Alexander Kläser univentionstaff 2012-10-19 22:06:37 CEST
Anbei eine erste Version, die es über eine neuen Methode ready() ermöglicht zu warten bis alle Lade-Aktionen fertig sind, zu einem beliebigen Zeitpunkt (Initialisierung oder später). ready() gibt ein Deferred-Objekt zurück. In meinen Tests hat dies dann auch mit UDM funktioniert.

In tests/depends.html wurde das Deferred, welches von der ready()-Method des Formulars zurückgegeben wurde, allerdings noch nicht komplett resolved, Test über:

> form._allReady.forEach(function(i, idx) { if (i && !i.isFulfilled()) { console.log(idx) } } )
Comment 7 Alexander Kläser univentionstaff 2012-10-25 16:10:55 CEST
Der Patch wurde noch etwas ausgebaut. Jetzt funktionieren ready auch wenn neue Werte gesetzt werden, die das Laden von neuen Listen-Werte auslösen. Das zurück gegebene Deferred wartet so lange bis das Listen-Widget aktualisiert wurde. Das UDM-UMC-Modul wurde auch leicht angepasst.


 univention-management-console-frontend (2.0.139-1) unstable; urgency=low
 .
   * added ready() methods to widgets for asynchronous notification; Bug #26214


 univention-management-console-module-udm (3.0.39-1) unstable; urgency=low
 .
   * use Form::ready(), adapted the modules startup handling; Bug #26214
Comment 8 Florian Best univentionstaff 2012-10-26 13:02:14 CEST
In Form.js
303 »   »   »   »   »   »   jwidget.ready ? jwidget.ready() : null;

fehlt hier nicht ein return?
Comment 9 Florian Best univentionstaff 2012-10-26 13:15:05 CEST
(In reply to comment #8)
> In Form.js
> 303 »   »   »   »   »   »   jwidget.ready ? jwidget.ready() : null;
> 
> fehlt hier nicht ein return?
Habe ich hinzugefügt in rev36673.
Comment 10 Dirk Wiesenthal univentionstaff 2012-11-13 18:16:02 CET
Der Fehler konnte nicht mehr reproduziert werden und insgesamt macht ready() einen sehr robusten Eindruck. Dummerweise ist das kein Beweis für irgend etwas in einer asynchronen Welt. Ich konnte in den Formularen, so wie wir sie nutzen, keinen Fehler feststellen, aber eine Analyse des Quellcodes lässt mich befürchten, dass wir nur "Glück" haben.

Der Bug wird REOPENED, nicht weil ich wirklich einen handfesten Fehler gefunden habe, sondern, weil ich die Aufmerksamkeit auf ein mögliches Problem lenken will (und der Bug soll dann - selbst wenn keine Änderungen mehr vorgenommen werden, weil das Problem nur theoretisch aufzutreten scheint - in diesem Bewusstsein geschlossen werden).

Das Problem sind meiner Meinung nach diese Zeilen in einigen Widgets:

if (this._readyDeferred.isFulfilled()) { this._readyDeferred = new Deferred(); }

Szenario:
form = new Form({widgets: widgets})
form.ready().then(function(){...})
-> Jede Menge Deferreds, man wartet auf alle.

In der Zwischenzeit aber dummerweise das (vielleicht "manuell" ausgeführt):
widgets[0]._loadValues() // new Deferred()

Dann erst:
widgets[0]._setValues() // deferred.resolve()

=> form.ready() mag ab hier sogar funktionieren, aber die erste Funktion würde meiner Meinung nach nie ausgeführt werden.


Kann sowas passieren? Kann mich wer beruhigen?
Comment 11 Dirk Wiesenthal univentionstaff 2012-11-13 18:24:07 CET
Außerdem fehlt ein Changelog Eintrag.
Comment 12 Alexander Kläser univentionstaff 2012-11-14 12:17:56 CET
(In reply to comment #10)
> Der Fehler konnte nicht mehr reproduziert werden und insgesamt macht ready()
> einen sehr robusten Eindruck. Dummerweise ist das kein Beweis für irgend etwas
> in einer asynchronen Welt. Ich konnte in den Formularen, so wie wir sie nutzen,
> keinen Fehler feststellen, aber eine Analyse des Quellcodes lässt mich
> befürchten, dass wir nur "Glück" haben.
> 
> Der Bug wird REOPENED, nicht weil ich wirklich einen handfesten Fehler gefunden
> habe, sondern, weil ich die Aufmerksamkeit auf ein mögliches Problem lenken
> will (und der Bug soll dann - selbst wenn keine Änderungen mehr vorgenommen
> werden, weil das Problem nur theoretisch aufzutreten scheint - in diesem
> Bewusstsein geschlossen werden).
> 
> Das Problem sind meiner Meinung nach diese Zeilen in einigen Widgets:

Guter Hinweis. Das sollte kein Problem sein... ich habe die nächsten Zeilen einmal kommentiert. Bitte noch einmal verifizieren.

> if (this._readyDeferred.isFulfilled()) { this._readyDeferred = new Deferred();
> }
> 
> Szenario:
> form = new Form({widgets: widgets})
> form.ready().then(function(){...})
> -> Jede Menge Deferreds, man wartet auf alle.
>
> In der Zwischenzeit aber dummerweise das (vielleicht "manuell" ausgeführt):
> widgets[0]._loadValues() // new Deferred()

(a) Wenn das readyDeferred beendet ist, würde ein neues erzeugt werden. → dann ist das readyDeferred in form.ready() aber bereits resolved und alles ist gut. Es wurde sicher gestellt (sollte zumindest), dass am Anfang eines Widgets das Deferred-Objekt angelegt wird, so dass es nach new Form(...) existiert (und nicht resolved ist, wenn etwas geladen wird).

(b) Wenn das Deferred noch nicht resolved ist, wird auch kein neues angelegt. Das ist ok. Das bisherige readyDeferred wird nicht überschrieben und in form.ready() wird weiterhin darauf gewartet.

Ist das ungefähr verständlich?

> Dann erst:
> widgets[0]._setValues() // deferred.resolve()
> 
> => form.ready() mag ab hier sogar funktionieren, aber die erste Funktion würde
> meiner Meinung nach nie ausgeführt werden.
> 
> 
> Kann sowas passieren? Kann mich wer beruhigen?

Sollte also alles ok sein :) .

Changelog-Eintrag wurde hinzugefügt.
Comment 13 Dirk Wiesenthal univentionstaff 2012-11-14 15:49:19 CET
Ist klar, verstanden. Erklärt auch, warum es funktioniert...
Comment 14 Stefan Gohmann univentionstaff 2012-12-12 21:09:02 CET
UCS 3.1-0 has been released: 
 http://forum.univention.de/viewtopic.php?f=54&t=2125

If this error occurs again, please use "Clone This Bug".