[live] SVG-Icons

So, das wäre dann das Update:

• The content cannot be displayed because you do not have authorisation to view this content.

Es war ja nur der unpassende Name, sonst nichts.

Du kannst auch warten, bis 0021 Large OSD buttons for large themes.patch im git ist, und dann anpassen.

Ich denke, die beiden überschneiden sich kaum. Nur eine Kleinigkeit in styles.css:

patching file live/css/styles.css
Hunk #1 succeeded at 2239 (offset 1 line).

-const cookieNamePrefix = "VDR-Live-Recordings-Tree";                                                                                                                
-const cookieNameOpenNodes = cookieNamePrefix + "-Open-Nodes";                                                                                                       
-const sessionStorageNameSelection = cookieNamePrefix + "-Selection";                                                                                                
+// please observe the systematic of the naming scheme                                                                                                               
+const liveNamePrefixRecTree = liveNamePrefix + "Recordings-Tree-";                                                                                                  
+const cookieNameOpenNodes = liveNamePrefixRecTree + "Open-Nodes";                                                                                                   
+const sessionStorageNameSelection = liveNamePrefixRecTree + "Selection";                                                                                            

Hat doch nichts mit "Browser-specific themes" zu tun. Nimmst Du das bitte raus, und machst dafür einen anderen Patch?

Es geht doch um die Konsistenz der Namensgebung… die jetzt übrigens außerhalb von treeview.js beginnt.

Der Patch aus #180 ist im git

Quote from MarkusE

Hat doch nichts mit "Browser-specific themes" zu tun. Nimmst Du das bitte raus, und machst dafür einen anderen Patch?

Quote from SHofmann

Es geht doch um die Konsistenz der Namensgebung… die jetzt übrigens außerhalb von treeview.js beginnt.

Lass mich nochmals versuchen zu erläutern, warum und wie ich die Namen strukturiert angepasst habe: Der grundlegende Gedanke war, dass wir für alle im Browser gespeicherten Daten nach einem einheitlichen Schema aufgebaute Bezeichner haben sollten:

<Prefix>-<Scope________>-<Purpose_>
VDR-Live-Recordings-Tree-Open-Nodes
VDR-Live-________________Local-Theme

[Die Unterstriche dienen nur der Tabulation, damit die einzelnen Bestandteile sauber untereinander angeordnet sind.]

Dabei entspricht <Scope> im Prinzip dem Namen der Seite, auf welcher der Bezeichner "exklusiv" verwendet wird, oder nutzt einen bereits früher eingeführten Scope; bei globalem Scope (wie für das lokale Theme) bleibt dieser Bestandteil leer. Anschließend beschreibt <Purpose> den eigentlichen Zweck. Dabei wird (wie bisher auch) nicht zwischen der Ablage in Session- oder Cookie-Storage unterschieden.

Die im Code verwendeten Makro- bzw. Variablennamen sollten, wie ich finde, eigentlich den gleichen Aufbau haben, hier aber noch den Storage mit beinhalten:

<Storage_____>Name<Purpose>
cookie________NameOpenNodes
sessionStorageNameSelection

Wenn der Scope klar ist, weil die Variablennamen etwa nur in einem eindeutigen Kontext verwendet werden – wie etwa treeview.js – kann man auf den Scope der Lesbarkeit halber auch verzichten.

Die Bezeichner werden, weil sie mit dem Patch nicht mehr nur in recordings.ecpp und treeview.js, sondern auch in pageelems.ecpp, login.ecpp und setup.ecpp verwendet werden, von zentraler Stelle aus hierarchisch zusammengefügt:

setup.h

 #define liveNamePrefix        "VDR-Live-"
 #define cookieNameLocalTheme  "Local-Theme"

Weil "VDR-Live-" generisch ist und noch keinem Storage-Typ zugeordnet ist, heißt der Makro liveNamePrefix. Weil er aber Bezeichner für Browser-Storages betrifft, hätte man natürlich auch noch etwas präziser sein und dafür beispielsweise liveStorageNamePrefix verwenden können.

Da das lokale Theme als Cookie mit globalem Scope (sprich: <Scope> = "") gespeichert werden soll und zudem in mehreren Dateien referenziert wird, ist es hier als cookieNameLocalTheme instantiiert.

login.ecpp, setup.ecpp

Generiert wird der Name des LocalTheme-Cookie (weil C++ eine solche String-Verkettung erlaubt) in den genannten Dateien per liveNamePrefix cookieNameLocalTheme, also ohne nochmals einen dedizierten Bezeichner zu erstellen. Man könnte einen solche natürlich dennoch deklarieren.

pageelems.ecpp

Hier wird das globale Präfix als Javascript-Konstante eingespeist, damit es in GetThemedLink() und treeview.js verwendet werden kann.

treeview.js

Hier befinden wir uns in den Recordings, sodass <Scope> = "RecTree". Entsprechend dem Schema werden die Variablennamen und Bezeichner wie folgt zusammengesetzt:

// please observe the systematic of the naming scheme
const liveNamePrefixRecTree = liveNamePrefix + "Recordings-Tree-";
const cookieNameOpenNodes = liveNamePrefixRecTree + "Open-Nodes";
const sessionStorageNameSelection = liveNamePrefixRecTree + "Selection";

… wobei liveNamePrefixRecTree nur behelfsweise definiert ist, um eine literale Redundanz in den anderen beiden Bezeichnern zu vermeiden

Wie du sieht, ist das Ganze weder aus Jux noch Tollerei entstanden, sondern folgt klaren Überlegungen und einem stringenten Aufbauschema. So ticke ich halt…

Doch auch wenn ich über die Jahre die Erfahrung gemacht habe, das (eine solche) Systematik das A und O wartbaren Codes ist, hängt mein Herz nicht an diesem Detail. Wenn du das also partout nicht haben willst, dann machen wir das halt anders.

Weil ich aber kein Gedankenleser bin und nicht noch etliche Iterationen haben möchte, böte es sich wohl an, dass du das gleich selbst erledigst.

> Doch auch wenn ich über die Jahre die Erfahrung gemacht habe, das (eine solche) Systematik das A und O wartbaren Codes ist, hängt mein Herz nicht an diesem Detail. Wenn du das also partout nicht haben willst, dann machen wird das halt anders.

Ich wollte doch nicht sagen, dass diese Änderung schlecht ist. Ich wollte doch nur sagen, dass sie nichts mit "Browser-specific themes" zu tun hat und daher nicht in diesen Patch gehört.

Ist aber jetzt nicht mehr relevant, ich habe den Patch in mein Testsystem übernommen, also brauche ich erst mal keine Änderungen mehr an dem Patch.

Da haben wir offenkundig unterschiedliche Sichtweisen. Wenn eine funktionale Änderung eine damit im Zusammenhang stehende Anpassung im Gesamtkontext des Projekts als sinnvoll oder wünschenswert erscheinen lässt, um Redundanzen und dergleichen zu vermeiden bzw. Konsistenz sicherzustellen, macht es nach meinem Dafürhalten keinen Sinn, an der einen Stelle zu ändern und an der anderen stur am Status quo festzuhalten.

Für mich hat die Refaktorisierung der Variablennamen also sehr wohl mit dem Thema des Patches zu tun.

Im git ist ein Update, mit dem Patch aus #183

Damit können Themes gerätespezifisch gewählt werden, dank SHofmann .

Ich dachte zwar, ich hätte das bei meinem Patch gelöst, aber ein kleines Problem haben wir noch. Dazu folgender Test:

• Theme "redwine" als globale Stilvorlage
• Browser 1: Theme "veltiner" als lokale Stilvorlage
• Browser 2: Theme "marine" als lokale Stilvorlage

Damit haben wir folgende Situation:

• Den VDR neu starten
• Browser 1: Reload mit Löschen des Cache (Firefox/Chromium: Shift + Reload-Icon)
• Browser 2: Reload mit Löschen des Cache (Firefox/Chromium: Shift + Reload-Icon)

Das führt dann hierzu:

Browser 2 nutzt jetzt nicht mehr das vorher eingestellte lokale Theme…

Löscht der Browser bei "Reload mit Löschen des Cache" auch das Cookie, in dem die lokale Stilvorlage gespeichert ist?

Nein, das tut er nicht. Der Grund scheint wohl zu sein, dass beim zweiten Browser der Abschnitt zum Auslesen des Cookies in login.ecpp nicht mehr aufgerufen wird. Keine Ahnung, warum die beiden Sessions nicht sauber entkoppelt sind.

Dieser Abschnitt hier:

  if (!logged_in && LiveSetup().UseAuth()) {
    cToSvConcat targetUrl = "/login.html?redirect=";
    targetUrl.appendUrlEscaped(request.getQuery());
    return reply.redirect(targetUrl.data());
  }

… lässt das Auslesen des Cookies für den zweiten Browser wohl ins leere laufen.

Wenn LiveSetup().UseAuth() auf "false" steht, greift der Mechanismus offenkundig nicht. Vielleicht sollte man diese Abfrage einfach herausnehmen und in in login.ecpp den Status von logged_in immer passend setzen. Damit würde login.ecpp zu Beginn jeder Session – wie eigentlich gedacht – wenigstens einmal aufgerufen. Gegebenenfalls muss man logged_in im Setup beim Ändern des Authentifizierungsverfahrens auf false setzen.

Ich muss jetzt aber außer Haus. Entweder magst du das selbst angehen oder ich kümmere mich morgen darum.

Ich habe eine grundsätzlich funktionierende Lösung gefunden. Das Problem war, wie vermutet, dass die Login-Seite nicht zuverlässig aufgerufen wurde und das Cookie mit dem lokalen Theme somit nicht ausgelesen wurde.

Die Lösung sieht im Groben so aus, dass ich (der leichteren Wartung und Konsistenz halber) die Codesequenz zum Aufruf der Login-Seite in eine Datei login.eh verlagert habe. Alle gemeinsamen globalen Session-Variablen liegen nun in page.eh, die nicht mehr automatisch von page_init.eh inkludiert wird.

Per login.eh wird immer die Login-Seite aufgerufen, wenn logged_in nicht gesetzt ist. Dort wird der Benutzer gegebenenfalls authentifiziert; ohne eingestellte Authentifizierung wird logged_in ohne weitere Prüfung auf true gesetzt. Sobald logged_in erstmalig gesetzt wird, wird das Cookie mit dem lokalen Theme ausgelesen:

// Browser 1
already -> logged_in = false -> true
got browserLocalTheme = veltliner
set browserLocalTheme = veltliner
browserLocalTheme = veltliner

// Browser 2
already -> logged_in = false -> true
got browserLocalTheme = marine
set browserLocalTheme = marine
browserLocalTheme = marine

Damit bekommt man bei obigem Test an die erwartete Anzeige:

Die Änderung muss ich jetzt natürlich noch gründlich für alle Seiten und Komponenten testen. Lass mich wissen, wenn du zeitnah einen Snapshot haben möchtest.

eilt nicht

Ich würde in page.eh

<%session scope="global">                                                                                                             
std::string effectiveTheme;                                                                                                        
</%session>

speichern. Dann kann man direkt in page.eh prüfen, ob effectiveTheme.empty() . Und falls ja, das Cookie auslesen und effectiveTheme bestimmen.

Normalerweise wird ja auch effectiveTheme und nicht browserLocalTheme benötigt.

Wenn doch browserLocalTheme benötigt wird (auf setup.ecpp), liest man es aus dem Cookie.

Erscheint mir irgendwie besser, ich kann das auch implementieren. Oder habe ich etwas übersehen?

Im git ist ein Update. Das in #193 sollte damit behoben sein.

Das wiederholte Auslesen und Schreiben des Cookies aus den Pages heraus funktioniert leider nicht so reibungslos, wie man sich das wünschen würde. Ein Versuch, das Cookie in den Einstellungen nach dem Schreiben wieder einzulesen und aufgrund einer Abweichung einen Fehler zu melden, hat leider gezeigt, dass getCookie() nicht das gerade eben per setCookie() geschriebene Theme liefert (obwohl dieses im Browser korrekt hinterlegt ist), sondern nach wie vor den alten Wert.

Deshalb halte ich es nach wie vor für günstiger, das Cookie nur einmalig beim Start der Session auszulesen und das effektive Theme daraus abzuleiten.

Der Commit funktioniert erwartungsgemäß. Allerdings habe ich gestern ziemlich viel Zeit darauf verwendet, den Aufruf des Login-Dialogs zu verbessern und zu vereinheitlichen. Soll ich die Änderungen noch an den neuen Commit anpassen und posten, oder möchtest du das lieber auf dem jetzigen Stand belassen?